Set StreamDataCountersCallback on construction of RTP modules
This CL sets the RTP stats callback on construction, by adding a field
next to the other observers in RtpRtcp::Configuration.
We can then remove the RegisterCallback() methods and the unused
GetCallback() method.
Bug: webrtc:11036
Change-Id: I4eb86ea63b4b2ebeff60b311ddf3bed06b279ce4
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/157169
Reviewed-by: Niels Moller <nisse@webrtc.org>
Commit-Queue: Erik Språng <sprang@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#29504}
diff --git a/call/rtp_video_sender.cc b/call/rtp_video_sender.cc
index 77a1338..4f62061 100644
--- a/call/rtp_video_sender.cc
+++ b/call/rtp_video_sender.cc
@@ -120,18 +120,12 @@
std::vector<RtpStreamSender> CreateRtpStreamSenders(
Clock* clock,
const RtpConfig& rtp_config,
+ const RtpSenderObservers& observers,
int rtcp_report_interval_ms,
Transport* send_transport,
- RtcpIntraFrameObserver* intra_frame_callback,
- RtcpLossNotificationObserver* rtcp_loss_notification_observer,
RtcpBandwidthObserver* bandwidth_callback,
RtpTransportControllerSendInterface* transport,
- RtcpRttStats* rtt_stats,
FlexfecSender* flexfec_sender,
- BitrateStatisticsObserver* bitrate_observer,
- RtcpPacketTypeCounterObserver* rtcp_type_observer,
- SendSideDelayObserver* send_delay_observer,
- SendPacketObserver* send_packet_observer,
RtcEventLog* event_log,
RateLimiter* retransmission_rate_limiter,
OverheadObserver* overhead_observer,
@@ -144,23 +138,25 @@
configuration.audio = false;
configuration.receiver_only = false;
configuration.outgoing_transport = send_transport;
- configuration.intra_frame_callback = intra_frame_callback;
+ configuration.intra_frame_callback = observers.intra_frame_callback;
configuration.rtcp_loss_notification_observer =
- rtcp_loss_notification_observer;
+ observers.rtcp_loss_notification_observer;
configuration.bandwidth_callback = bandwidth_callback;
configuration.network_state_estimate_observer =
transport->network_state_estimate_observer();
configuration.transport_feedback_callback =
transport->transport_feedback_observer();
- configuration.rtt_stats = rtt_stats;
- configuration.rtcp_packet_type_counter_observer = rtcp_type_observer;
+ configuration.rtt_stats = observers.rtcp_rtt_stats;
+ configuration.rtcp_packet_type_counter_observer =
+ observers.rtcp_type_observer;
configuration.paced_sender = transport->packet_sender();
- configuration.send_bitrate_observer = bitrate_observer;
- configuration.send_side_delay_observer = send_delay_observer;
- configuration.send_packet_observer = send_packet_observer;
+ configuration.send_bitrate_observer = observers.bitrate_observer;
+ configuration.send_side_delay_observer = observers.send_delay_observer;
+ configuration.send_packet_observer = observers.send_packet_observer;
configuration.event_log = event_log;
configuration.retransmission_rate_limiter = retransmission_rate_limiter;
configuration.overhead_observer = overhead_observer;
+ configuration.rtp_stats_callback = observers.rtp_stats;
configuration.frame_encryptor = frame_encryptor;
configuration.require_frame_encryption =
crypto_options.sframe.require_frame_encryption;
@@ -320,26 +316,19 @@
MaybeCreateFlexfecSender(clock, rtp_config, suspended_ssrcs_)),
fec_controller_(std::move(fec_controller)),
fec_allowed_(true),
- rtp_streams_(
- CreateRtpStreamSenders(clock,
- rtp_config,
- rtcp_report_interval_ms,
- send_transport,
- observers.intra_frame_callback,
- observers.rtcp_loss_notification_observer,
- transport->GetBandwidthObserver(),
- transport,
- observers.rtcp_rtt_stats,
- flexfec_sender_.get(),
- observers.bitrate_observer,
- observers.rtcp_type_observer,
- observers.send_delay_observer,
- observers.send_packet_observer,
- event_log,
- retransmission_limiter,
- this,
- frame_encryptor,
- crypto_options)),
+ rtp_streams_(CreateRtpStreamSenders(clock,
+ rtp_config,
+ observers,
+ rtcp_report_interval_ms,
+ send_transport,
+ transport->GetBandwidthObserver(),
+ transport,
+ flexfec_sender_.get(),
+ event_log,
+ retransmission_limiter,
+ this,
+ frame_encryptor,
+ crypto_options)),
rtp_config_(rtp_config),
codec_type_(GetVideoCodecType(rtp_config)),
transport_(transport),
@@ -400,8 +389,6 @@
stream.rtp_rtcp->RegisterRtcpStatisticsCallback(observers.rtcp_stats);
stream.rtp_rtcp->SetReportBlockDataObserver(
observers.report_block_data_observer);
- stream.rtp_rtcp->RegisterSendChannelRtpStatisticsCallback(
- observers.rtp_stats);
stream.rtp_rtcp->SetMaxRtpPacketSize(rtp_config_.max_packet_size);
stream.rtp_rtcp->RegisterSendPayloadFrequency(rtp_config_.payload_type,
kVideoPayloadTypeFrequency);
diff --git a/modules/rtp_rtcp/include/rtp_rtcp.h b/modules/rtp_rtcp/include/rtp_rtcp.h
index 185f9e8..7682b4a 100644
--- a/modules/rtp_rtcp/include/rtp_rtcp.h
+++ b/modules/rtp_rtcp/include/rtp_rtcp.h
@@ -102,6 +102,7 @@
RateLimiter* retransmission_rate_limiter = nullptr;
OverheadObserver* overhead_observer = nullptr;
RtcpAckObserver* ack_observer = nullptr;
+ StreamDataCountersCallback* rtp_stats_callback = nullptr;
int rtcp_report_interval_ms = 0;
@@ -275,12 +276,6 @@
virtual std::vector<std::unique_ptr<RtpPacketToSend>> GeneratePadding(
size_t target_size_bytes) = 0;
- // Called on generation of new statistics after an RTP send.
- virtual void RegisterSendChannelRtpStatisticsCallback(
- StreamDataCountersCallback* callback) = 0;
- virtual StreamDataCountersCallback* GetSendChannelRtpStatisticsCallback()
- const = 0;
-
// **************************************************************************
// RTCP
// **************************************************************************
diff --git a/modules/rtp_rtcp/mocks/mock_rtp_rtcp.h b/modules/rtp_rtcp/mocks/mock_rtp_rtcp.h
index 3b64de7..5b81fe1 100644
--- a/modules/rtp_rtcp/mocks/mock_rtp_rtcp.h
+++ b/modules/rtp_rtcp/mocks/mock_rtp_rtcp.h
@@ -160,10 +160,6 @@
bool decodability_flag,
bool buffering_allowed));
MOCK_METHOD0(Process, void());
- MOCK_METHOD1(RegisterSendChannelRtpStatisticsCallback,
- void(StreamDataCountersCallback*));
- MOCK_CONST_METHOD0(GetSendChannelRtpStatisticsCallback,
- StreamDataCountersCallback*());
MOCK_METHOD1(SetVideoBitrateAllocation, void(const VideoBitrateAllocation&));
MOCK_METHOD0(RtpSender, RTPSender*());
MOCK_CONST_METHOD0(RtpSender, const RTPSender*());
diff --git a/modules/rtp_rtcp/source/rtp_rtcp_impl.cc b/modules/rtp_rtcp/source/rtp_rtcp_impl.cc
index ed140ee..4ff584e 100644
--- a/modules/rtp_rtcp/source/rtp_rtcp_impl.cc
+++ b/modules/rtp_rtcp/source/rtp_rtcp_impl.cc
@@ -741,16 +741,6 @@
return rtt_ms_;
}
-void ModuleRtpRtcpImpl::RegisterSendChannelRtpStatisticsCallback(
- StreamDataCountersCallback* callback) {
- rtp_sender_->RegisterRtpStatisticsCallback(callback);
-}
-
-StreamDataCountersCallback*
-ModuleRtpRtcpImpl::GetSendChannelRtpStatisticsCallback() const {
- return rtp_sender_->GetRtpStatisticsCallback();
-}
-
void ModuleRtpRtcpImpl::SetVideoBitrateAllocation(
const VideoBitrateAllocation& bitrate) {
rtcp_sender_.SetVideoBitrateAllocation(bitrate);
diff --git a/modules/rtp_rtcp/source/rtp_rtcp_impl.h b/modules/rtp_rtcp/source/rtp_rtcp_impl.h
index e91f704..2d6cfff 100644
--- a/modules/rtp_rtcp/source/rtp_rtcp_impl.h
+++ b/modules/rtp_rtcp/source/rtp_rtcp_impl.h
@@ -260,11 +260,6 @@
uint32_t* fec_rate,
uint32_t* nackRate) const override;
- void RegisterSendChannelRtpStatisticsCallback(
- StreamDataCountersCallback* callback) override;
- StreamDataCountersCallback* GetSendChannelRtpStatisticsCallback()
- const override;
-
void OnReceivedNack(
const std::vector<uint16_t>& nack_sequence_numbers) override;
void OnReceivedRtcpReportBlocks(
diff --git a/modules/rtp_rtcp/source/rtp_sender.cc b/modules/rtp_rtcp/source/rtp_sender.cc
index 05afcec..c9555fa 100644
--- a/modules/rtp_rtcp/source/rtp_sender.cc
+++ b/modules/rtp_rtcp/source/rtp_sender.cc
@@ -146,7 +146,7 @@
max_delay_it_(send_delays_.end()),
sum_delays_ms_(0),
total_packet_send_delay_ms_(0),
- rtp_stats_callback_(nullptr),
+ rtp_stats_callback_(config.rtp_stats_callback),
total_bitrate_sent_(kBitrateStatisticsWindowMs,
RateStatistics::kBpsScale),
nack_bitrate_sent_(kBitrateStatisticsWindowMs, RateStatistics::kBpsScale),
@@ -1071,17 +1071,6 @@
return rtx_packet;
}
-void RTPSender::RegisterRtpStatisticsCallback(
- StreamDataCountersCallback* callback) {
- rtc::CritScope cs(&statistics_crit_);
- rtp_stats_callback_ = callback;
-}
-
-StreamDataCountersCallback* RTPSender::GetRtpStatisticsCallback() const {
- rtc::CritScope cs(&statistics_crit_);
- return rtp_stats_callback_;
-}
-
uint32_t RTPSender::BitrateSent() const {
rtc::CritScope cs(&statistics_crit_);
return total_bitrate_sent_.Rate(clock_->TimeInMilliseconds()).value_or(0);
diff --git a/modules/rtp_rtcp/source/rtp_sender.h b/modules/rtp_rtcp/source/rtp_sender.h
index bed2bba..28512b8 100644
--- a/modules/rtp_rtcp/source/rtp_sender.h
+++ b/modules/rtp_rtcp/source/rtp_sender.h
@@ -153,10 +153,6 @@
// sending to the network.
void EnqueuePackets(std::vector<std::unique_ptr<RtpPacketToSend>> packets);
- // Called on update of RTP statistics.
- void RegisterRtpStatisticsCallback(StreamDataCountersCallback* callback);
- StreamDataCountersCallback* GetRtpStatisticsCallback() const;
-
uint32_t BitrateSent() const;
void SetRtpState(const RtpState& rtp_state);
@@ -254,8 +250,7 @@
uint64_t total_packet_send_delay_ms_ RTC_GUARDED_BY(statistics_crit_);
StreamDataCounters rtp_stats_ RTC_GUARDED_BY(statistics_crit_);
StreamDataCounters rtx_rtp_stats_ RTC_GUARDED_BY(statistics_crit_);
- StreamDataCountersCallback* rtp_stats_callback_
- RTC_GUARDED_BY(statistics_crit_);
+ StreamDataCountersCallback* const rtp_stats_callback_;
RateStatistics total_bitrate_sent_ RTC_GUARDED_BY(statistics_crit_);
RateStatistics nack_bitrate_sent_ RTC_GUARDED_BY(statistics_crit_);
SendSideDelayObserver* const send_side_delay_observer_;
diff --git a/modules/rtp_rtcp/source/rtp_sender_unittest.cc b/modules/rtp_rtcp/source/rtp_sender_unittest.cc
index 0b2d48e..1cd3ea4 100644
--- a/modules/rtp_rtcp/source/rtp_sender_unittest.cc
+++ b/modules/rtp_rtcp/source/rtp_sender_unittest.cc
@@ -189,6 +189,37 @@
MOCK_METHOD1(OnOverheadChanged, void(size_t overhead_bytes_per_packet));
};
+class StreamDataTestCallback : public StreamDataCountersCallback {
+ public:
+ StreamDataTestCallback()
+ : StreamDataCountersCallback(), ssrc_(0), counters_() {}
+ ~StreamDataTestCallback() override = default;
+
+ void DataCountersUpdated(const StreamDataCounters& counters,
+ uint32_t ssrc) override {
+ ssrc_ = ssrc;
+ counters_ = counters;
+ }
+
+ uint32_t ssrc_;
+ StreamDataCounters counters_;
+
+ void MatchPacketCounter(const RtpPacketCounter& expected,
+ const RtpPacketCounter& actual) {
+ EXPECT_EQ(expected.payload_bytes, actual.payload_bytes);
+ EXPECT_EQ(expected.header_bytes, actual.header_bytes);
+ EXPECT_EQ(expected.padding_bytes, actual.padding_bytes);
+ EXPECT_EQ(expected.packets, actual.packets);
+ }
+
+ void Matches(uint32_t ssrc, const StreamDataCounters& counters) {
+ EXPECT_EQ(ssrc, ssrc_);
+ MatchPacketCounter(counters.transmitted, counters_.transmitted);
+ MatchPacketCounter(counters.retransmitted, counters_.retransmitted);
+ EXPECT_EQ(counters.fec.packets, counters_.fec.packets);
+ }
+};
+
class RtpSenderTest : public ::testing::TestWithParam<TestConfig> {
protected:
RtpSenderTest()
@@ -223,6 +254,7 @@
config.retransmission_rate_limiter = &retransmission_rate_limiter_;
config.paced_sender = pacer ? &mock_paced_sender_ : nullptr;
config.populate_network2_timestamp = populate_network2;
+ config.rtp_stats_callback = &rtp_stats_callback_;
rtp_sender_.reset(new RTPSender(config));
rtp_sender_->SetSequenceNumber(kSeqNum);
rtp_sender_->SetTimestampOffset(0);
@@ -239,6 +271,7 @@
LoopbackTransportTest transport_;
const bool kMarkerBit;
test::ScopedFieldTrials field_trials_;
+ StreamDataTestCallback rtp_stats_callback_;
std::unique_ptr<RtpPacketToSend> BuildRtpPacket(int payload_type,
bool marker_bit,
@@ -1804,40 +1837,7 @@
rtp_sender_.reset();
}
-class StreamDataTestCallback : public StreamDataCountersCallback {
- public:
- StreamDataTestCallback()
- : StreamDataCountersCallback(), ssrc_(0), counters_() {}
- ~StreamDataTestCallback() override = default;
-
- void DataCountersUpdated(const StreamDataCounters& counters,
- uint32_t ssrc) override {
- ssrc_ = ssrc;
- counters_ = counters;
- }
-
- uint32_t ssrc_;
- StreamDataCounters counters_;
-
- void MatchPacketCounter(const RtpPacketCounter& expected,
- const RtpPacketCounter& actual) {
- EXPECT_EQ(expected.payload_bytes, actual.payload_bytes);
- EXPECT_EQ(expected.header_bytes, actual.header_bytes);
- EXPECT_EQ(expected.padding_bytes, actual.padding_bytes);
- EXPECT_EQ(expected.packets, actual.packets);
- }
-
- void Matches(uint32_t ssrc, const StreamDataCounters& counters) {
- EXPECT_EQ(ssrc, ssrc_);
- MatchPacketCounter(counters.transmitted, counters_.transmitted);
- MatchPacketCounter(counters.retransmitted, counters_.retransmitted);
- EXPECT_EQ(counters.fec.packets, counters_.fec.packets);
- }
-};
-
TEST_P(RtpSenderTestWithoutPacer, StreamDataCountersCallbacks) {
- StreamDataTestCallback callback;
-
const uint8_t kPayloadType = 127;
const VideoCodecType kCodecType = VideoCodecType::kVideoCodecGeneric;
PlayoutDelayOracle playout_delay_oracle;
@@ -1852,8 +1852,6 @@
rtp_sender_->SetStorePacketsStatus(true, 1);
uint32_t ssrc = rtp_sender_->SSRC();
- rtp_sender_->RegisterRtpStatisticsCallback(&callback);
-
// Send a frame.
RTPVideoHeader video_header;
video_header.frame_type = VideoFrameType::kVideoFrameKey;
@@ -1870,7 +1868,7 @@
expected.retransmitted.padding_bytes = 0;
expected.retransmitted.packets = 0;
expected.fec.packets = 0;
- callback.Matches(ssrc, expected);
+ rtp_stats_callback_.Matches(ssrc, expected);
// Retransmit a frame.
uint16_t seqno = rtp_sender_->SequenceNumber() - 1;
@@ -1882,7 +1880,7 @@
expected.retransmitted.header_bytes = 12;
expected.retransmitted.padding_bytes = 0;
expected.retransmitted.packets = 1;
- callback.Matches(ssrc, expected);
+ rtp_stats_callback_.Matches(ssrc, expected);
// Send padding.
GenerateAndSendPadding(kMaxPaddingSize);
@@ -1890,14 +1888,10 @@
expected.transmitted.header_bytes = 36;
expected.transmitted.padding_bytes = kMaxPaddingSize;
expected.transmitted.packets = 3;
- callback.Matches(ssrc, expected);
-
- rtp_sender_->RegisterRtpStatisticsCallback(nullptr);
+ rtp_stats_callback_.Matches(ssrc, expected);
}
TEST_P(RtpSenderTestWithoutPacer, StreamDataCountersCallbacksUlpfec) {
- StreamDataTestCallback callback;
-
const uint8_t kRedPayloadType = 96;
const uint8_t kUlpfecPayloadType = 97;
const uint8_t kPayloadType = 127;
@@ -1916,8 +1910,6 @@
rtp_sender_->SetStorePacketsStatus(true, 1);
uint32_t ssrc = rtp_sender_->SSRC();
- rtp_sender_->RegisterRtpStatisticsCallback(&callback);
-
RTPVideoHeader video_header;
StreamDataCounters expected;
@@ -1935,9 +1927,7 @@
expected.transmitted.header_bytes = 24;
expected.transmitted.packets = 2;
expected.fec.packets = 1;
- callback.Matches(ssrc, expected);
-
- rtp_sender_->RegisterRtpStatisticsCallback(nullptr);
+ rtp_stats_callback_.Matches(ssrc, expected);
}
TEST_P(RtpSenderTestWithoutPacer, BytesReportedCorrectly) {
