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}
diff --git a/modules/rtp_rtcp/include/rtcp_statistics.h b/modules/rtp_rtcp/include/rtcp_statistics.h
index e78a875..e26c475 100644
--- a/modules/rtp_rtcp/include/rtcp_statistics.h
+++ b/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 @@
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 @@
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_
diff --git a/modules/rtp_rtcp/include/rtp_rtcp.h b/modules/rtp_rtcp/include/rtp_rtcp.h
index 0ff6753..00b49f2 100644
--- a/modules/rtp_rtcp/include/rtp_rtcp.h
+++ b/modules/rtp_rtcp/include/rtp_rtcp.h
@@ -426,9 +426,13 @@
// 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(
diff --git a/modules/rtp_rtcp/mocks/mock_rtp_rtcp.h b/modules/rtp_rtcp/mocks/mock_rtp_rtcp.h
index 1cb488c..1a9a0c4 100644
--- a/modules/rtp_rtcp/mocks/mock_rtp_rtcp.h
+++ b/modules/rtp_rtcp/mocks/mock_rtp_rtcp.h
@@ -153,6 +153,7 @@
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,
diff --git a/modules/rtp_rtcp/source/receive_statistics_unittest.cc b/modules/rtp_rtcp/source/receive_statistics_unittest.cc
index 6a8ba66..a06754e 100644
--- a/modules/rtp_rtcp/source/receive_statistics_unittest.cc
+++ b/modules/rtp_rtcp/source/receive_statistics_unittest.cc
@@ -247,8 +247,6 @@
++num_calls_;
}
- void CNameChanged(const char* cname, uint32_t ssrc) override {}
-
uint32_t num_calls_;
uint32_t ssrc_;
RtcpStatistics stats_;
diff --git a/modules/rtp_rtcp/source/rtcp_receiver.cc b/modules/rtp_rtcp/source/rtcp_receiver.cc
index 7754ab1..69cb44f 100644
--- a/modules/rtp_rtcp/source/rtcp_receiver.cc
+++ b/modules/rtp_rtcp/source/rtcp_receiver.cc
@@ -146,6 +146,7 @@
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 @@
received_cnames_[chunk.ssrc] = chunk.cname;
{
rtc::CritScope lock(&feedbacks_lock_);
- if (stats_callback_)
- stats_callback_->CNameChanged(chunk.cname.c_str(), chunk.ssrc);
+ if (cname_callback_)
+ cname_callback_->OnCname(chunk.ssrc, chunk.cname);
}
}
packet_information->packet_type_flags |= kRtcpSdes;
@@ -1000,6 +1001,11 @@
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_);
diff --git a/modules/rtp_rtcp/source/rtcp_receiver.h b/modules/rtp_rtcp/source/rtcp_receiver.h
index 7d684cb..3056711 100644
--- a/modules/rtp_rtcp/source/rtcp_receiver.h
+++ b/modules/rtp_rtcp/source/rtcp_receiver.h
@@ -111,6 +111,7 @@
void NotifyTmmbrUpdated();
void RegisterRtcpStatisticsCallback(RtcpStatisticsCallback* callback);
+ void RegisterRtcpCnameCallback(RtcpCnameCallback* callback);
RtcpStatisticsCallback* GetRtcpStatisticsCallback();
void SetReportBlockDataObserver(ReportBlockDataObserver* observer);
@@ -265,6 +266,7 @@
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.
diff --git a/modules/rtp_rtcp/source/rtcp_receiver_unittest.cc b/modules/rtp_rtcp/source/rtcp_receiver_unittest.cc
index 8a2a89e..e9c6e2c 100644
--- a/modules/rtp_rtcp/source/rtcp_receiver_unittest.cc
+++ b/modules/rtp_rtcp/source/rtcp_receiver_unittest.cc
@@ -84,7 +84,11 @@
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, InjectSdesWithOneChunk) {
const char kCname[] = "alice@host";
- MockRtcpCallbackImpl callback;
- rtcp_receiver_.RegisterRtcpStatisticsCallback(&callback);
+ MockCnameCallbackImpl 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];
diff --git a/modules/rtp_rtcp/source/rtp_rtcp_impl.cc b/modules/rtp_rtcp/source/rtp_rtcp_impl.cc
index 2aed84e..5c6697e 100644
--- a/modules/rtp_rtcp/source/rtp_rtcp_impl.cc
+++ b/modules/rtp_rtcp/source/rtp_rtcp_impl.cc
@@ -666,6 +666,10 @@
return rtcp_receiver_.GetRtcpStatisticsCallback();
}
+void ModuleRtpRtcpImpl::RegisterRtcpCnameCallback(RtcpCnameCallback* callback) {
+ rtcp_receiver_.RegisterRtcpCnameCallback(callback);
+}
+
void ModuleRtpRtcpImpl::SetReportBlockDataObserver(
ReportBlockDataObserver* observer) {
return rtcp_receiver_.SetReportBlockDataObserver(observer);
diff --git a/modules/rtp_rtcp/source/rtp_rtcp_impl.h b/modules/rtp_rtcp/source/rtp_rtcp_impl.h
index 1c8ef9c..b34b145 100644
--- a/modules/rtp_rtcp/source/rtp_rtcp_impl.h
+++ b/modules/rtp_rtcp/source/rtp_rtcp_impl.h
@@ -242,6 +242,8 @@
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;
diff --git a/video/receive_statistics_proxy.cc b/video/receive_statistics_proxy.cc
index 158146a..ea88846 100644
--- a/video/receive_statistics_proxy.cc
+++ b/video/receive_statistics_proxy.cc
@@ -670,13 +670,13 @@
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(
diff --git a/video/receive_statistics_proxy.h b/video/receive_statistics_proxy.h
index 289efb9..adfab02 100644
--- a/video/receive_statistics_proxy.h
+++ b/video/receive_statistics_proxy.h
@@ -38,6 +38,7 @@
class ReceiveStatisticsProxy : public VCMReceiveStatisticsCallback,
public RtcpStatisticsCallback,
+ public RtcpCnameCallback,
public RtcpPacketTypeCounterObserver,
public StreamDataCountersCallback,
public CallStatsObserver {
@@ -80,7 +81,8 @@
// 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(
diff --git a/video/receive_statistics_proxy_unittest.cc b/video/receive_statistics_proxy_unittest.cc
index c629067..ce622eb 100644
--- a/video/receive_statistics_proxy_unittest.cc
+++ b/video/receive_statistics_proxy_unittest.cc
@@ -485,13 +485,13 @@
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());
}
diff --git a/video/rtp_video_stream_receiver.cc b/video/rtp_video_stream_receiver.cc
index 2fd4f50..9e7ae23 100644
--- a/video/rtp_video_stream_receiver.cc
+++ b/video/rtp_video_stream_receiver.cc
@@ -225,7 +225,7 @@
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);
diff --git a/video/send_statistics_proxy.cc b/video/send_statistics_proxy.cc
index 72dd514..c2aab05 100644
--- a/video/send_statistics_proxy.cc
+++ b/video/send_statistics_proxy.cc
@@ -1161,8 +1161,6 @@
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_);
diff --git a/video/send_statistics_proxy.h b/video/send_statistics_proxy.h
index 30e8f8b..5280aad 100644
--- a/video/send_statistics_proxy.h
+++ b/video/send_statistics_proxy.h
@@ -95,7 +95,6 @@
// 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.
