diff --git a/call/rtp_video_sender.cc b/call/rtp_video_sender.cc index 77a13382f7..4f62061742 100644 --- a/call/rtp_video_sender.cc +++ b/call/rtp_video_sender.cc @@ -120,18 +120,12 @@ bool ShouldDisableRedAndUlpfec(bool flexfec_enabled, std::vector 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 @@ std::vector CreateRtpStreamSenders( 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 @@ RtpVideoSender::RtpVideoSender( 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 @@ RtpVideoSender::RtpVideoSender( 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 185f9e8d0f..7682b4a628 100644 --- a/modules/rtp_rtcp/include/rtp_rtcp.h +++ b/modules/rtp_rtcp/include/rtp_rtcp.h @@ -102,6 +102,7 @@ class RtpRtcp : public Module, public RtcpFeedbackSenderInterface { 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 @@ class RtpRtcp : public Module, public RtcpFeedbackSenderInterface { virtual std::vector> 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 3b64de7fe3..5b81fe18b2 100644 --- a/modules/rtp_rtcp/mocks/mock_rtp_rtcp.h +++ b/modules/rtp_rtcp/mocks/mock_rtp_rtcp.h @@ -160,10 +160,6 @@ class MockRtpRtcp : public RtpRtcp { 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 ed140eeb04..4ff584e27f 100644 --- a/modules/rtp_rtcp/source/rtp_rtcp_impl.cc +++ b/modules/rtp_rtcp/source/rtp_rtcp_impl.cc @@ -741,16 +741,6 @@ int64_t ModuleRtpRtcpImpl::rtt_ms() const { 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 e91f70404d..2d6cfff489 100644 --- a/modules/rtp_rtcp/source/rtp_rtcp_impl.h +++ b/modules/rtp_rtcp/source/rtp_rtcp_impl.h @@ -260,11 +260,6 @@ class ModuleRtpRtcpImpl : public RtpRtcp, public RTCPReceiver::ModuleRtpRtcp { uint32_t* fec_rate, uint32_t* nackRate) const override; - void RegisterSendChannelRtpStatisticsCallback( - StreamDataCountersCallback* callback) override; - StreamDataCountersCallback* GetSendChannelRtpStatisticsCallback() - const override; - void OnReceivedNack( const std::vector& 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 05afcec02a..c9555fa767 100644 --- a/modules/rtp_rtcp/source/rtp_sender.cc +++ b/modules/rtp_rtcp/source/rtp_sender.cc @@ -146,7 +146,7 @@ RTPSender::RTPSender(const RtpRtcp::Configuration& config) 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 @@ std::unique_ptr RTPSender::BuildRtxPacket( 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 bed2bba630..28512b81ad 100644 --- a/modules/rtp_rtcp/source/rtp_sender.h +++ b/modules/rtp_rtcp/source/rtp_sender.h @@ -153,10 +153,6 @@ class RTPSender { // sending to the network. void EnqueuePackets(std::vector> 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 @@ class RTPSender { 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 0b2d48e9db..1cd3ea46c0 100644 --- a/modules/rtp_rtcp/source/rtp_sender_unittest.cc +++ b/modules/rtp_rtcp/source/rtp_sender_unittest.cc @@ -189,6 +189,37 @@ class MockOverheadObserver : public OverheadObserver { 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 { protected: RtpSenderTest() @@ -223,6 +254,7 @@ class RtpSenderTest : public ::testing::TestWithParam { 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 @@ class RtpSenderTest : public ::testing::TestWithParam { LoopbackTransportTest transport_; const bool kMarkerBit; test::ScopedFieldTrials field_trials_; + StreamDataTestCallback rtp_stats_callback_; std::unique_ptr BuildRtpPacket(int payload_type, bool marker_bit, @@ -1804,40 +1837,7 @@ TEST_P(RtpSenderTest, BitrateCallbacks) { 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 @@ TEST_P(RtpSenderTestWithoutPacer, StreamDataCountersCallbacks) { 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 @@ TEST_P(RtpSenderTestWithoutPacer, StreamDataCountersCallbacks) { 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 @@ TEST_P(RtpSenderTestWithoutPacer, StreamDataCountersCallbacks) { 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 @@ TEST_P(RtpSenderTestWithoutPacer, StreamDataCountersCallbacks) { 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 @@ TEST_P(RtpSenderTestWithoutPacer, StreamDataCountersCallbacksUlpfec) { rtp_sender_->SetStorePacketsStatus(true, 1); uint32_t ssrc = rtp_sender_->SSRC(); - rtp_sender_->RegisterRtpStatisticsCallback(&callback); - RTPVideoHeader video_header; StreamDataCounters expected; @@ -1935,9 +1927,7 @@ TEST_P(RtpSenderTestWithoutPacer, StreamDataCountersCallbacksUlpfec) { 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) {