diff --git a/audio/channel_receive.cc b/audio/channel_receive.cc index f61f298ae3..801c39ea6e 100644 --- a/audio/channel_receive.cc +++ b/audio/channel_receive.cc @@ -521,8 +521,6 @@ ChannelReceive::~ChannelReceive() { media_transport_->SetReceiveAudioSink(nullptr); } - rtp_receive_statistics_->RegisterRtcpStatisticsCallback(NULL); - StopPlayout(); int error = audio_coding_->RegisterTransportCallback(NULL); diff --git a/modules/rtp_rtcp/include/receive_statistics.h b/modules/rtp_rtcp/include/receive_statistics.h index ab68ee0f69..f905eb15f1 100644 --- a/modules/rtp_rtcp/include/receive_statistics.h +++ b/modules/rtp_rtcp/include/receive_statistics.h @@ -12,6 +12,7 @@ #define MODULES_RTP_RTCP_INCLUDE_RECEIVE_STATISTICS_H_ #include +#include #include #include "call/rtp_packet_sink_interface.h" @@ -54,7 +55,14 @@ class ReceiveStatistics : public ReceiveStatisticsProvider, public: ~ReceiveStatistics() override = default; - static ReceiveStatistics* Create(Clock* clock); + static ReceiveStatistics* Create(Clock* clock) { + return Create(clock, nullptr, nullptr).release(); + } + + static std::unique_ptr Create( + Clock* clock, + RtcpStatisticsCallback* rtcp_callback, + StreamDataCountersCallback* rtp_callback); // Increment counter for number of FEC packets received. virtual void FecPacketReceived(const RtpPacketReceived& packet) = 0; @@ -67,14 +75,6 @@ class ReceiveStatistics : public ReceiveStatisticsProvider, // Detect retransmissions, enabling updates of the retransmitted counters. The // default is false. virtual void EnableRetransmitDetection(uint32_t ssrc, bool enable) = 0; - - // Called on new RTCP stats creation. - virtual void RegisterRtcpStatisticsCallback( - RtcpStatisticsCallback* callback) = 0; - - // Called on new RTP stats creation. - virtual void RegisterRtpStatisticsCallback( - StreamDataCountersCallback* callback) = 0; }; } // namespace webrtc diff --git a/modules/rtp_rtcp/source/receive_statistics_impl.cc b/modules/rtp_rtcp/source/receive_statistics_impl.cc index 30a0f360f6..0869d88ebb 100644 --- a/modules/rtp_rtcp/source/receive_statistics_impl.cc +++ b/modules/rtp_rtcp/source/receive_statistics_impl.cc @@ -11,10 +11,11 @@ #include "modules/rtp_rtcp/source/receive_statistics_impl.h" #include - #include +#include #include +#include "absl/memory/memory.h" #include "modules/remote_bitrate_estimator/test/bwe_test_logging.h" #include "modules/rtp_rtcp/source/rtp_packet_received.h" #include "modules/rtp_rtcp/source/rtp_rtcp_config.h" @@ -59,7 +60,8 @@ StreamStatisticianImpl::~StreamStatisticianImpl() = default; void StreamStatisticianImpl::OnRtpPacket(const RtpPacketReceived& packet) { StreamDataCounters counters = UpdateCounters(packet); - rtp_callback_->DataCountersUpdated(counters, ssrc_); + if (rtp_callback_) + rtp_callback_->DataCountersUpdated(counters, ssrc_); } StreamDataCounters StreamStatisticianImpl::UpdateCounters( @@ -146,7 +148,8 @@ void StreamStatisticianImpl::FecPacketReceived( receive_counters_.fec.AddPacket(packet); counters = receive_counters_; } - rtp_callback_->DataCountersUpdated(counters, ssrc_); + if (rtp_callback_) + rtp_callback_->DataCountersUpdated(counters, ssrc_); } void StreamStatisticianImpl::SetMaxReorderingThreshold( @@ -183,7 +186,8 @@ bool StreamStatisticianImpl::GetStatistics(RtcpStatistics* statistics, *statistics = CalculateRtcpStatistics(); } - rtcp_callback_->StatisticsUpdated(*statistics, ssrc_); + if (rtcp_callback_) + rtcp_callback_->StatisticsUpdated(*statistics, ssrc_); return true; } @@ -205,7 +209,8 @@ bool StreamStatisticianImpl::GetActiveStatisticsAndReset( *statistics = CalculateRtcpStatistics(); } - rtcp_callback_->StatisticsUpdated(*statistics, ssrc_); + if (rtcp_callback_) + rtcp_callback_->StatisticsUpdated(*statistics, ssrc_); return true; } @@ -334,16 +339,23 @@ bool StreamStatisticianImpl::IsRetransmitOfOldPacket( return time_diff_ms > rtp_time_stamp_diff_ms + max_delay_ms; } -ReceiveStatistics* ReceiveStatistics::Create(Clock* clock) { - return new ReceiveStatisticsImpl(clock); +std::unique_ptr ReceiveStatistics::Create( + Clock* clock, + RtcpStatisticsCallback* rtcp_callback, + StreamDataCountersCallback* rtp_callback) { + return absl::make_unique(clock, rtcp_callback, + rtp_callback); } -ReceiveStatisticsImpl::ReceiveStatisticsImpl(Clock* clock) +ReceiveStatisticsImpl::ReceiveStatisticsImpl( + Clock* clock, + RtcpStatisticsCallback* rtcp_callback, + StreamDataCountersCallback* rtp_callback) : clock_(clock), last_returned_ssrc_(0), max_reordering_threshold_(kDefaultMaxReorderingThreshold), - rtcp_stats_callback_(NULL), - rtp_stats_callback_(NULL) {} + rtcp_stats_callback_(rtcp_callback), + rtp_stats_callback_(rtp_callback) {} ReceiveStatisticsImpl::~ReceiveStatisticsImpl() { while (!statisticians_.empty()) { @@ -362,7 +374,7 @@ void ReceiveStatisticsImpl::OnRtpPacket(const RtpPacketReceived& packet) { } else { impl = new StreamStatisticianImpl( packet.Ssrc(), clock_, /* enable_retransmit_detection = */ false, - max_reordering_threshold_, this, this); + max_reordering_threshold_, rtcp_stats_callback_, rtp_stats_callback_); statisticians_[packet.Ssrc()] = impl; } } @@ -416,7 +428,8 @@ void ReceiveStatisticsImpl::EnableRetransmitDetection(uint32_t ssrc, StreamStatisticianImpl*& impl_ref = statisticians_[ssrc]; if (impl_ref == nullptr) { // new element impl_ref = new StreamStatisticianImpl( - ssrc, clock_, enable, max_reordering_threshold_, this, this); + ssrc, clock_, enable, max_reordering_threshold_, rtcp_stats_callback_, + rtp_stats_callback_); return; } impl = impl_ref; @@ -424,43 +437,6 @@ void ReceiveStatisticsImpl::EnableRetransmitDetection(uint32_t ssrc, impl->EnableRetransmitDetection(enable); } -void ReceiveStatisticsImpl::RegisterRtcpStatisticsCallback( - RtcpStatisticsCallback* callback) { - rtc::CritScope cs(&receive_statistics_lock_); - if (callback != NULL) - assert(rtcp_stats_callback_ == NULL); - rtcp_stats_callback_ = callback; -} - -void ReceiveStatisticsImpl::StatisticsUpdated(const RtcpStatistics& statistics, - uint32_t ssrc) { - rtc::CritScope cs(&receive_statistics_lock_); - if (rtcp_stats_callback_) - rtcp_stats_callback_->StatisticsUpdated(statistics, ssrc); -} - -void ReceiveStatisticsImpl::CNameChanged(const char* cname, uint32_t ssrc) { - rtc::CritScope cs(&receive_statistics_lock_); - if (rtcp_stats_callback_) - rtcp_stats_callback_->CNameChanged(cname, ssrc); -} - -void ReceiveStatisticsImpl::RegisterRtpStatisticsCallback( - StreamDataCountersCallback* callback) { - rtc::CritScope cs(&receive_statistics_lock_); - if (callback != NULL) - assert(rtp_stats_callback_ == NULL); - rtp_stats_callback_ = callback; -} - -void ReceiveStatisticsImpl::DataCountersUpdated(const StreamDataCounters& stats, - uint32_t ssrc) { - rtc::CritScope cs(&receive_statistics_lock_); - if (rtp_stats_callback_) { - rtp_stats_callback_->DataCountersUpdated(stats, ssrc); - } -} - std::vector ReceiveStatisticsImpl::RtcpReportBlocks( size_t max_blocks) { std::map statisticians; diff --git a/modules/rtp_rtcp/source/receive_statistics_impl.h b/modules/rtp_rtcp/source/receive_statistics_impl.h index f6aec69ffb..2a9ea93f77 100644 --- a/modules/rtp_rtcp/source/receive_statistics_impl.h +++ b/modules/rtp_rtcp/source/receive_statistics_impl.h @@ -94,11 +94,11 @@ class StreamStatisticianImpl : public StreamStatistician, StreamDataCountersCallback* const rtp_callback_; }; -class ReceiveStatisticsImpl : public ReceiveStatistics, - public RtcpStatisticsCallback, - public StreamDataCountersCallback { +class ReceiveStatisticsImpl : public ReceiveStatistics { public: - explicit ReceiveStatisticsImpl(Clock* clock); + ReceiveStatisticsImpl(Clock* clock, + RtcpStatisticsCallback* rtcp_callback, + StreamDataCountersCallback* rtp_callback); ~ReceiveStatisticsImpl() override; @@ -114,19 +114,7 @@ class ReceiveStatisticsImpl : public ReceiveStatistics, void SetMaxReorderingThreshold(int max_reordering_threshold) override; void EnableRetransmitDetection(uint32_t ssrc, bool enable) override; - void RegisterRtcpStatisticsCallback( - RtcpStatisticsCallback* callback) override; - - void RegisterRtpStatisticsCallback( - StreamDataCountersCallback* callback) override; - private: - void StatisticsUpdated(const RtcpStatistics& statistics, - uint32_t ssrc) override; - void CNameChanged(const char* cname, uint32_t ssrc) override; - void DataCountersUpdated(const StreamDataCounters& counters, - uint32_t ssrc) override; - Clock* const clock_; rtc::CriticalSection receive_statistics_lock_; uint32_t last_returned_ssrc_; @@ -134,8 +122,8 @@ class ReceiveStatisticsImpl : public ReceiveStatistics, std::map statisticians_ RTC_GUARDED_BY(receive_statistics_lock_); - RtcpStatisticsCallback* rtcp_stats_callback_; - StreamDataCountersCallback* rtp_stats_callback_; + RtcpStatisticsCallback* const rtcp_stats_callback_; + StreamDataCountersCallback* const rtp_stats_callback_; }; } // namespace webrtc #endif // MODULES_RTP_RTCP_SOURCE_RECEIVE_STATISTICS_IMPL_H_ diff --git a/modules/rtp_rtcp/source/receive_statistics_unittest.cc b/modules/rtp_rtcp/source/receive_statistics_unittest.cc index 578d81fc3d..25393631df 100644 --- a/modules/rtp_rtcp/source/receive_statistics_unittest.cc +++ b/modules/rtp_rtcp/source/receive_statistics_unittest.cc @@ -71,7 +71,9 @@ void IncrementTimestamp(RtpPacketReceived* packet, uint32_t incr) { class ReceiveStatisticsTest : public ::testing::Test { public: ReceiveStatisticsTest() - : clock_(0), receive_statistics_(ReceiveStatistics::Create(&clock_)) { + : clock_(0), + receive_statistics_( + ReceiveStatistics::Create(&clock_, nullptr, nullptr)) { packet1_ = CreateRtpPacket(kSsrc1, kPacketSize1); packet2_ = CreateRtpPacket(kSsrc2, kPacketSize2); } @@ -251,7 +253,7 @@ TEST_F(ReceiveStatisticsTest, RtcpCallbacks) { RtcpStatistics stats_; } callback; - receive_statistics_->RegisterRtcpStatisticsCallback(&callback); + receive_statistics_ = ReceiveStatistics::Create(&clock_, &callback, nullptr); receive_statistics_->EnableRetransmitDetection(kSsrc1, true); // Add some arbitrary data, with loss and jitter. @@ -291,33 +293,6 @@ TEST_F(ReceiveStatisticsTest, RtcpCallbacks) { EXPECT_EQ(1, statistics.packets_lost); EXPECT_EQ(5u, statistics.extended_highest_sequence_number); EXPECT_EQ(177u, statistics.jitter); - - receive_statistics_->RegisterRtcpStatisticsCallback(NULL); - - // Add some more data. - packet1_.SetSequenceNumber(1); - clock_.AdvanceTimeMilliseconds(7); - IncrementTimestamp(&packet1_, 3); - receive_statistics_->OnRtpPacket(packet1_); - IncrementSequenceNumber(&packet1_, 2); - clock_.AdvanceTimeMilliseconds(9); - IncrementTimestamp(&packet1_, 9); - receive_statistics_->OnRtpPacket(packet1_); - IncrementSequenceNumber(&packet1_, -1); - clock_.AdvanceTimeMilliseconds(13); - IncrementTimestamp(&packet1_, 47); - receive_statistics_->OnRtpPacket(packet1_); - IncrementSequenceNumber(&packet1_, 3); - clock_.AdvanceTimeMilliseconds(11); - IncrementTimestamp(&packet1_, 17); - receive_statistics_->OnRtpPacket(packet1_); - IncrementSequenceNumber(&packet1_); - - receive_statistics_->GetStatistician(kSsrc1)->GetStatistics(&statistics, - true); - - // Should not have been called after deregister. - EXPECT_EQ(1u, callback.num_calls_); } class RtpTestCallback : public StreamDataCountersCallback { @@ -358,7 +333,7 @@ class RtpTestCallback : public StreamDataCountersCallback { TEST_F(ReceiveStatisticsTest, RtpCallbacks) { RtpTestCallback callback; - receive_statistics_->RegisterRtpStatisticsCallback(&callback); + receive_statistics_ = ReceiveStatistics::Create(&clock_, nullptr, &callback); receive_statistics_->EnableRetransmitDetection(kSsrc1, true); const size_t kHeaderLength = 20; @@ -417,19 +392,11 @@ TEST_F(ReceiveStatisticsTest, RtpCallbacks) { expected.fec.header_bytes = kHeaderLength; expected.fec.packets = 1; callback.Matches(5, kSsrc1, expected); - - receive_statistics_->RegisterRtpStatisticsCallback(NULL); - - // New stats, but callback should not be called. - IncrementSequenceNumber(&packet1); - clock_.AdvanceTimeMilliseconds(5); - receive_statistics_->OnRtpPacket(packet1); - callback.Matches(5, kSsrc1, expected); } TEST_F(ReceiveStatisticsTest, RtpCallbacksFecFirst) { RtpTestCallback callback; - receive_statistics_->RegisterRtpStatisticsCallback(&callback); + receive_statistics_ = ReceiveStatistics::Create(&clock_, nullptr, &callback); const uint32_t kHeaderLength = 20; RtpPacketReceived packet = diff --git a/video/rtp_video_stream_receiver.cc b/video/rtp_video_stream_receiver.cc index 3ebddf0e0e..fdf02ff329 100644 --- a/video/rtp_video_stream_receiver.cc +++ b/video/rtp_video_stream_receiver.cc @@ -117,8 +117,6 @@ RtpVideoStreamReceiver::RtpVideoStreamReceiver( frame_decryptor_(frame_decryptor) { constexpr bool remb_candidate = true; packet_router_->AddReceiveRtpModule(rtp_rtcp_.get(), remb_candidate); - rtp_receive_statistics_->RegisterRtpStatisticsCallback(receive_stats_proxy); - rtp_receive_statistics_->RegisterRtcpStatisticsCallback(receive_stats_proxy); RTC_DCHECK(config_.rtp.rtcp_mode != RtcpMode::kOff) << "A stream should not be configured with RTCP disabled. This value is " diff --git a/video/video_receive_stream.cc b/video/video_receive_stream.cc index 4f2b9df752..bba33aa363 100644 --- a/video/video_receive_stream.cc +++ b/video/video_receive_stream.cc @@ -135,10 +135,11 @@ VideoReceiveStream::VideoReceiveStream( "DecodingThread", rtc::kHighestPriority), call_stats_(call_stats), - rtp_receive_statistics_(ReceiveStatistics::Create(clock_)), + stats_proxy_(&config_, clock_), + rtp_receive_statistics_( + ReceiveStatistics::Create(clock_, &stats_proxy_, &stats_proxy_)), timing_(new VCMTiming(clock_)), video_receiver_(clock_, timing_.get(), this, this), - stats_proxy_(&config_, clock_), rtp_video_stream_receiver_(&transport_adapter_, call_stats, packet_router, diff --git a/video/video_receive_stream.h b/video/video_receive_stream.h index e32aa1900a..5c9fd83c62 100644 --- a/video/video_receive_stream.h +++ b/video/video_receive_stream.h @@ -114,6 +114,7 @@ class VideoReceiveStream : public webrtc::VideoReceiveStream, CallStats* const call_stats_; + ReceiveStatisticsProxy stats_proxy_; // Shared by media and rtx stream receivers, since the latter has no RtpRtcp // module of its own. const std::unique_ptr rtp_receive_statistics_; @@ -121,7 +122,6 @@ class VideoReceiveStream : public webrtc::VideoReceiveStream, std::unique_ptr timing_; // Jitter buffer experiment. vcm::VideoReceiver video_receiver_; std::unique_ptr> incoming_video_stream_; - ReceiveStatisticsProxy stats_proxy_; RtpVideoStreamReceiver rtp_video_stream_receiver_; std::unique_ptr video_stream_decoder_; RtpStreamsSynchronizer rtp_stream_sync_;