Split out RtcpCnameCallback from RtcpStatisticsCallback

Cname callback is used only on receive side, and statistics (soon)
only on the send side.

Bug: webrtc:10679
Change-Id: I122e9cafaea93cd0ba75dc955a652d9d4bddc379
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/147867
Reviewed-by: Erik Språng <sprang@webrtc.org>
Reviewed-by: Danil Chapovalov <danilchap@webrtc.org>
Commit-Queue: Niels Moller <nisse@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#28767}

modules/rtp_rtcp/include/rtcp_statistics.h

@@ -13,6 +13,8 @@
 
 #include <stdint.h>
 
+#include "absl/strings/string_view.h"
+
 namespace webrtc {
 
 // Statistics for an RTCP channel
@@ -29,7 +31,6 @@ class RtcpStatisticsCallback {
 
   virtual void StatisticsUpdated(const RtcpStatistics& statistics,
                                  uint32_t ssrc) = 0;
-  virtual void CNameChanged(const char* cname, uint32_t ssrc) = 0;
 };
 
 // Statistics for RTCP packet types.
@@ -98,5 +99,13 @@ class RtcpPacketTypeCounterObserver {
       const RtcpPacketTypeCounter& packet_counter) = 0;
 };
 
+// Invoked for each cname passed in RTCP SDES blocks.
+class RtcpCnameCallback {
+ public:
+  virtual ~RtcpCnameCallback() = default;
+
+  virtual void OnCname(uint32_t ssrc, absl::string_view cname) = 0;
+};
+
 }  // namespace webrtc
 #endif  // MODULES_RTP_RTCP_INCLUDE_RTCP_STATISTICS_H_

modules/rtp_rtcp/include/rtp_rtcp.h

@@ -426,9 +426,13 @@ class RtpRtcp : public Module, public RtcpFeedbackSenderInterface {
   // getters or only callbacks. If we decide on getters, the
   // ReportBlockDataObserver should also be removed in favor of
   // GetLatestReportBlockData().
+  // TODO(nisse): Replace RegisterRtcpStatisticsCallback and
+  // RegisterRtcpCnameCallback with construction-time settings in
+  // RtpRtcp::Configuration.
   virtual void RegisterRtcpStatisticsCallback(
       RtcpStatisticsCallback* callback) = 0;
   virtual RtcpStatisticsCallback* GetRtcpStatisticsCallback() = 0;
+  virtual void RegisterRtcpCnameCallback(RtcpCnameCallback* callback) = 0;
   // TODO(https://crbug.com/webrtc/10680): When callbacks are registered at
   // construction, remove this setter.
   virtual void SetReportBlockDataObserver(

modules/rtp_rtcp/mocks/mock_rtp_rtcp.h

@@ -153,6 +153,7 @@ class MockRtpRtcp : public RtpRtcp {
   MOCK_CONST_METHOD0(StorePackets, bool());
   MOCK_METHOD1(RegisterRtcpStatisticsCallback, void(RtcpStatisticsCallback*));
   MOCK_METHOD0(GetRtcpStatisticsCallback, RtcpStatisticsCallback*());
+  MOCK_METHOD1(RegisterRtcpCnameCallback, void(RtcpCnameCallback*));
   MOCK_METHOD1(SetReportBlockDataObserver, void(ReportBlockDataObserver*));
   MOCK_METHOD1(SendFeedbackPacket, bool(const rtcp::TransportFeedback& packet));
   MOCK_METHOD1(SendNetworkStateEstimatePacket,

modules/rtp_rtcp/source/receive_statistics_unittest.cc

@@ -247,8 +247,6 @@ TEST_F(ReceiveStatisticsTest, RtcpCallbacks) {
       ++num_calls_;
     }
 
-    void CNameChanged(const char* cname, uint32_t ssrc) override {}
-
     uint32_t num_calls_;
     uint32_t ssrc_;
     RtcpStatistics stats_;

modules/rtp_rtcp/source/rtcp_receiver.cc

@@ -146,6 +146,7 @@ RTCPReceiver::RTCPReceiver(const RtpRtcp::Configuration& config,
       last_received_rb_ms_(0),
       last_increased_sequence_number_ms_(0),
       stats_callback_(nullptr),
+      cname_callback_(nullptr),
       report_block_data_observer_(nullptr),
       packet_type_counter_observer_(config.rtcp_packet_type_counter_observer),
       num_skipped_packets_(0),
@@ -664,8 +665,8 @@ void RTCPReceiver::HandleSdes(const CommonHeader& rtcp_block,
     received_cnames_[chunk.ssrc] = chunk.cname;
     {
       rtc::CritScope lock(&feedbacks_lock_);
-      if (stats_callback_)
+      if (cname_callback_)
-        stats_callback_->CNameChanged(chunk.cname.c_str(), chunk.ssrc);
+        cname_callback_->OnCname(chunk.ssrc, chunk.cname);
     }
   }
   packet_information->packet_type_flags |= kRtcpSdes;
@@ -1000,6 +1001,11 @@ RtcpStatisticsCallback* RTCPReceiver::GetRtcpStatisticsCallback() {
   return stats_callback_;
 }
 
+void RTCPReceiver::RegisterRtcpCnameCallback(RtcpCnameCallback* callback) {
+  rtc::CritScope cs(&feedbacks_lock_);
+  cname_callback_ = callback;
+}
+
 void RTCPReceiver::SetReportBlockDataObserver(
     ReportBlockDataObserver* observer) {
   rtc::CritScope cs(&feedbacks_lock_);

modules/rtp_rtcp/source/rtcp_receiver.h

@@ -111,6 +111,7 @@ class RTCPReceiver {
   void NotifyTmmbrUpdated();
 
   void RegisterRtcpStatisticsCallback(RtcpStatisticsCallback* callback);
+  void RegisterRtcpCnameCallback(RtcpCnameCallback* callback);
   RtcpStatisticsCallback* GetRtcpStatisticsCallback();
   void SetReportBlockDataObserver(ReportBlockDataObserver* observer);
 
@@ -265,6 +266,7 @@ class RTCPReceiver {
   int64_t last_increased_sequence_number_ms_;
 
   RtcpStatisticsCallback* stats_callback_ RTC_GUARDED_BY(feedbacks_lock_);
+  RtcpCnameCallback* cname_callback_ RTC_GUARDED_BY(feedbacks_lock_);
   // TODO(hbos): Remove RtcpStatisticsCallback in favor of
   // ReportBlockDataObserver; the ReportBlockData contains a superset of the
   // RtcpStatistics data.

modules/rtp_rtcp/source/rtcp_receiver_unittest.cc

@@ -84,7 +84,11 @@ class MockRtcpLossNotificationObserver : public RtcpLossNotificationObserver {
 class MockRtcpCallbackImpl : public RtcpStatisticsCallback {
  public:
   MOCK_METHOD2(StatisticsUpdated, void(const RtcpStatistics&, uint32_t));
-  MOCK_METHOD2(CNameChanged, void(const char*, uint32_t));
+};
+
+class MockCnameCallbackImpl : public RtcpCnameCallback {
+ public:
+  MOCK_METHOD2(OnCname, void(uint32_t, absl::string_view));
 };
 
 class MockReportBlockDataObserverImpl : public ReportBlockDataObserver {
@@ -584,12 +588,12 @@ TEST_F(RtcpReceiverTest, InjectApp) {
 
 TEST_F(RtcpReceiverTest, InjectSdesWithOneChunk) {
   const char kCname[] = "alice@host";
-  MockRtcpCallbackImpl callback;
+  MockCnameCallbackImpl callback;
-  rtcp_receiver_.RegisterRtcpStatisticsCallback(&callback);
+  rtcp_receiver_.RegisterRtcpCnameCallback(&callback);
   rtcp::Sdes sdes;
   sdes.AddCName(kSenderSsrc, kCname);
 
-  EXPECT_CALL(callback, CNameChanged(StrEq(kCname), kSenderSsrc));
+  EXPECT_CALL(callback, OnCname(kSenderSsrc, StrEq(kCname)));
   InjectRtcpPacket(sdes);
 
   char cName[RTCP_CNAME_SIZE];

modules/rtp_rtcp/source/rtp_rtcp_impl.cc

@@ -666,6 +666,10 @@ RtcpStatisticsCallback* ModuleRtpRtcpImpl::GetRtcpStatisticsCallback() {
   return rtcp_receiver_.GetRtcpStatisticsCallback();
 }
 
+void ModuleRtpRtcpImpl::RegisterRtcpCnameCallback(RtcpCnameCallback* callback) {
+  rtcp_receiver_.RegisterRtcpCnameCallback(callback);
+}
+
 void ModuleRtpRtcpImpl::SetReportBlockDataObserver(
     ReportBlockDataObserver* observer) {
   return rtcp_receiver_.SetReportBlockDataObserver(observer);

modules/rtp_rtcp/source/rtp_rtcp_impl.h

@@ -242,6 +242,8 @@ class ModuleRtpRtcpImpl : public RtpRtcp, public RTCPReceiver::ModuleRtpRtcp {
   void RegisterRtcpStatisticsCallback(
       RtcpStatisticsCallback* callback) override;
   RtcpStatisticsCallback* GetRtcpStatisticsCallback() override;
+  void RegisterRtcpCnameCallback(RtcpCnameCallback* callback) override;
+
   void SetReportBlockDataObserver(ReportBlockDataObserver* observer) override;
 
   bool SendFeedbackPacket(const rtcp::TransportFeedback& packet) override;

video/receive_statistics_proxy.cc

@@ -670,13 +670,13 @@ void ReceiveStatisticsProxy::StatisticsUpdated(
   stats_.rtcp_stats = statistics;
 }
 
-void ReceiveStatisticsProxy::CNameChanged(const char* cname, uint32_t ssrc) {
+void ReceiveStatisticsProxy::OnCname(uint32_t ssrc, absl::string_view cname) {
   rtc::CritScope lock(&crit_);
   // TODO(pbos): Handle both local and remote ssrcs here and RTC_DCHECK that we
   // receive stats from one of them.
   if (stats_.ssrc != ssrc)
     return;
-  stats_.c_name = cname;
+  stats_.c_name = std::string(cname);
 }
 
 void ReceiveStatisticsProxy::DataCountersUpdated(

video/receive_statistics_proxy.h

@@ -38,6 +38,7 @@ struct CodecSpecificInfo;
 
 class ReceiveStatisticsProxy : public VCMReceiveStatisticsCallback,
                                public RtcpStatisticsCallback,
+                               public RtcpCnameCallback,
                                public RtcpPacketTypeCounterObserver,
                                public StreamDataCountersCallback,
                                public CallStatsObserver {
@@ -80,7 +81,8 @@ class ReceiveStatisticsProxy : public VCMReceiveStatisticsCallback,
   // Overrides RtcpStatisticsCallback.
   void StatisticsUpdated(const webrtc::RtcpStatistics& statistics,
                          uint32_t ssrc) override;
-  void CNameChanged(const char* cname, uint32_t ssrc) override;
+  // Overrides RtcpCnameCallback.
+  void OnCname(uint32_t ssrc, absl::string_view cname) override;
 
   // Overrides RtcpPacketTypeCounterObserver.
   void RtcpPacketTypesCounterUpdated(

video/receive_statistics_proxy_unittest.cc

@@ -485,13 +485,13 @@ TEST_F(ReceiveStatisticsProxyTest, GetStatsReportsRtcpStats) {
 
 TEST_F(ReceiveStatisticsProxyTest, GetStatsReportsCName) {
   const char* kName = "cName";
-  statistics_proxy_->CNameChanged(kName, kRemoteSsrc);
+  statistics_proxy_->OnCname(kRemoteSsrc, kName);
   EXPECT_STREQ(kName, statistics_proxy_->GetStats().c_name.c_str());
 }
 
 TEST_F(ReceiveStatisticsProxyTest, GetStatsReportsNoCNameForUnknownSsrc) {
   const char* kName = "cName";
-  statistics_proxy_->CNameChanged(kName, kRemoteSsrc + 1);
+  statistics_proxy_->OnCname(kRemoteSsrc + 1, kName);
   EXPECT_STREQ("", statistics_proxy_->GetStats().c_name.c_str());
 }
 

video/rtp_video_stream_receiver.cc

@@ -225,7 +225,7 @@ RtpVideoStreamReceiver::RtpVideoStreamReceiver(
     rtp_rtcp_->SetRtcpXrRrtrStatus(true);
 
   // Stats callback for CNAME changes.
-  rtp_rtcp_->RegisterRtcpStatisticsCallback(receive_stats_proxy);
+  rtp_rtcp_->RegisterRtcpCnameCallback(receive_stats_proxy);
 
   process_thread_->RegisterModule(rtp_rtcp_.get(), RTC_FROM_HERE);
 

video/send_statistics_proxy.cc

@@ -1161,8 +1161,6 @@ void SendStatisticsProxy::StatisticsUpdated(const RtcpStatistics& statistics,
   uma_container_->report_block_stats_.Store(ssrc, statistics);
 }
 
-void SendStatisticsProxy::CNameChanged(const char* cname, uint32_t ssrc) {}
-
 void SendStatisticsProxy::OnReportBlockDataUpdated(
     ReportBlockData report_block_data) {
   rtc::CritScope lock(&crit_);

video/send_statistics_proxy.h

@@ -95,7 +95,6 @@ class SendStatisticsProxy : public VideoStreamEncoderObserver,
   // From RtcpStatisticsCallback.
   void StatisticsUpdated(const RtcpStatistics& statistics,
                          uint32_t ssrc) override;
-  void CNameChanged(const char* cname, uint32_t ssrc) override;
   // From ReportBlockDataObserver.
   void OnReportBlockDataUpdated(ReportBlockData report_block_data) override;
   // From RtcpPacketTypeCounterObserver.