From 3e39254b678fb175c3029255b285fefeb8caebe9 Mon Sep 17 00:00:00 2001 From: Danil Chapovalov Date: Wed, 17 May 2023 13:25:39 +0200 Subject: [PATCH] Pass rtcp message to RtpTransportController through newer interface MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit NetworkLinkRtcpObserver is similar to RtcpBandwidthObserver but pass time variables using unit types instead of raw integers. Bug: webrtc:13757 Change-Id: Iaa0bbe0b108620b3a24013c40e7d9004032e904d Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/305022 Reviewed-by: Jakob Ivarsson‎ Commit-Queue: Danil Chapovalov Reviewed-by: Rasmus Brandt Cr-Commit-Position: refs/heads/main@{#40087} --- audio/audio_send_stream_unittest.cc | 8 +-- audio/channel_send.cc | 4 +- call/rtp_transport_controller_send.cc | 50 +++++++++---------- call/rtp_transport_controller_send.h | 23 +++++---- .../rtp_transport_controller_send_interface.h | 3 +- call/rtp_video_sender.cc | 4 +- .../test/mock_rtp_transport_controller_send.h | 2 +- 7 files changed, 45 insertions(+), 49 deletions(-) diff --git a/audio/audio_send_stream_unittest.cc b/audio/audio_send_stream_unittest.cc index 9afcfda0a6..d842afdfe5 100644 --- a/audio/audio_send_stream_unittest.cc +++ b/audio/audio_send_stream_unittest.cc @@ -28,7 +28,7 @@ #include "modules/audio_mixer/sine_wave_generator.h" #include "modules/audio_processing/include/audio_processing_statistics.h" #include "modules/audio_processing/include/mock_audio_processing.h" -#include "modules/rtp_rtcp/mocks/mock_rtcp_bandwidth_observer.h" +#include "modules/rtp_rtcp/mocks/mock_network_link_rtcp_observer.h" #include "modules/rtp_rtcp/mocks/mock_rtp_rtcp.h" #include "system_wrappers/include/clock.h" #include "test/gtest.h" @@ -225,8 +225,8 @@ struct ConfigHelper { EXPECT_CALL(*channel_send_, SetSendAudioLevelIndicationStatus(true, kAudioLevelId)) .Times(1); - EXPECT_CALL(rtp_transport_, GetBandwidthObserver()) - .WillRepeatedly(Return(&bandwidth_observer_)); + EXPECT_CALL(rtp_transport_, GetRtcpObserver) + .WillRepeatedly(Return(&rtcp_observer_)); if (audio_bwe_enabled) { EXPECT_CALL(rtp_rtcp_, RegisterRtpHeaderExtension(TransportSequenceNumber::Uri(), @@ -323,7 +323,7 @@ struct ConfigHelper { ::testing::StrictMock* channel_send_ = nullptr; rtc::scoped_refptr audio_processing_; AudioProcessingStats audio_processing_stats_; - ::testing::StrictMock bandwidth_observer_; + ::testing::StrictMock rtcp_observer_; ::testing::NiceMock event_log_; ::testing::NiceMock rtp_transport_; ::testing::NiceMock rtp_rtcp_; diff --git a/audio/channel_send.cc b/audio/channel_send.cc index c893f9a61c..fccd58b76c 100644 --- a/audio/channel_send.cc +++ b/audio/channel_send.cc @@ -396,8 +396,8 @@ ChannelSend::ChannelSend( RtpRtcpInterface::Configuration configuration; configuration.report_block_data_observer = this; - configuration.bandwidth_callback = - transport_controller->GetBandwidthObserver(); + configuration.network_link_rtcp_observer = + transport_controller->GetRtcpObserver(); configuration.transport_feedback_callback = transport_controller->transport_feedback_observer(); configuration.clock = (clock ? clock : Clock::GetRealTimeClock()); diff --git a/call/rtp_transport_controller_send.cc b/call/rtp_transport_controller_send.cc index c86e667185..f2d6cd0567 100644 --- a/call/rtp_transport_controller_send.cc +++ b/call/rtp_transport_controller_send.cc @@ -354,7 +354,7 @@ void RtpTransportControllerSend::OnNetworkAvailability(bool network_available) { rtp_sender->OnNetworkAvailability(network_available); } } -RtcpBandwidthObserver* RtpTransportControllerSend::GetBandwidthObserver() { +NetworkLinkRtcpObserver* RtpTransportControllerSend::GetRtcpObserver() { return this; } int64_t RtpTransportControllerSend::GetPacerQueuingDelayMs() const { @@ -507,24 +507,23 @@ void RtpTransportControllerSend::EnsureStarted() { } } -void RtpTransportControllerSend::OnReceivedEstimatedBitrate(uint32_t bitrate) { +void RtpTransportControllerSend::OnReceiverEstimatedMaxBitrate( + Timestamp receive_time, + DataRate bitrate) { RTC_DCHECK_RUN_ON(&sequence_checker_); RemoteBitrateReport msg; - msg.receive_time = Timestamp::Millis(clock_->TimeInMilliseconds()); - msg.bandwidth = DataRate::BitsPerSec(bitrate); + msg.receive_time = receive_time; + msg.bandwidth = bitrate; if (controller_) PostUpdates(controller_->OnRemoteBitrateReport(msg)); } -void RtpTransportControllerSend::OnReceivedRtcpReceiverReport( - const ReportBlockList& report_blocks, - int64_t rtt_ms, - int64_t now_ms) { +void RtpTransportControllerSend::OnRttUpdate(Timestamp receive_time, + TimeDelta rtt) { RTC_DCHECK_RUN_ON(&sequence_checker_); - OnReceivedRtcpReceiverReportBlocks(report_blocks, now_ms); RoundTripTimeUpdate report; - report.receive_time = Timestamp::Millis(now_ms); - report.round_trip_time = TimeDelta::Millis(rtt_ms); + report.receive_time = receive_time; + report.round_trip_time = rtt; report.smoothed = false; if (controller_ && !report.round_trip_time.IsZero()) PostUpdates(controller_->OnRoundTripTimeUpdate(report)); @@ -540,13 +539,13 @@ void RtpTransportControllerSend::OnAddPacket( } void RtpTransportControllerSend::OnTransportFeedback( + Timestamp receive_time, const rtcp::TransportFeedback& feedback) { RTC_DCHECK_RUN_ON(&sequence_checker_); - auto feedback_time = Timestamp::Millis(clock_->TimeInMilliseconds()); feedback_demuxer_.OnTransportFeedback(feedback); absl::optional feedback_msg = transport_feedback_adapter_.ProcessTransportFeedback(feedback, - feedback_time); + receive_time); if (feedback_msg) { if (controller_) PostUpdates(controller_->OnTransportPacketsFeedback(*feedback_msg)); @@ -658,9 +657,9 @@ void RtpTransportControllerSend::PostUpdates(NetworkControlUpdate update) { } } -void RtpTransportControllerSend::OnReceivedRtcpReceiverReportBlocks( - const ReportBlockList& report_blocks, - int64_t now_ms) { +void RtpTransportControllerSend::OnReport( + Timestamp receive_time, + rtc::ArrayView report_blocks) { RTC_DCHECK_RUN_ON(&sequence_checker_); if (report_blocks.empty()) return; @@ -669,19 +668,19 @@ void RtpTransportControllerSend::OnReceivedRtcpReceiverReportBlocks( int total_packets_delta = 0; // Compute the packet loss from all report blocks. - for (const RTCPReportBlock& report_block : report_blocks) { + for (const ReportBlockData& report_block : report_blocks) { auto [it, inserted] = - last_report_blocks_.try_emplace(report_block.source_ssrc); + last_report_blocks_.try_emplace(report_block.source_ssrc()); LossReport& last_loss_report = it->second; if (!inserted) { - total_packets_delta += report_block.extended_highest_sequence_number - + total_packets_delta += report_block.extended_highest_sequence_number() - last_loss_report.extended_highest_sequence_number; total_packets_lost_delta += - report_block.packets_lost - last_loss_report.cumulative_lost; + report_block.cumulative_lost() - last_loss_report.cumulative_lost; } last_loss_report.extended_highest_sequence_number = - report_block.extended_highest_sequence_number; - last_loss_report.cumulative_lost = report_block.packets_lost; + report_block.extended_highest_sequence_number(); + last_loss_report.cumulative_lost = report_block.cumulative_lost(); } // Can only compute delta if there has been previous blocks to compare to. If // not, total_packets_delta will be unchanged and there's nothing more to do. @@ -694,16 +693,15 @@ void RtpTransportControllerSend::OnReceivedRtcpReceiverReportBlocks( if (packets_received_delta < 1) return; - Timestamp now = Timestamp::Millis(now_ms); TransportLossReport msg; msg.packets_lost_delta = total_packets_lost_delta; msg.packets_received_delta = packets_received_delta; - msg.receive_time = now; + msg.receive_time = receive_time; msg.start_time = last_report_block_time_; - msg.end_time = now; + msg.end_time = receive_time; if (controller_) PostUpdates(controller_->OnTransportLossReport(msg)); - last_report_block_time_ = now; + last_report_block_time_ = receive_time; } } // namespace webrtc diff --git a/call/rtp_transport_controller_send.h b/call/rtp_transport_controller_send.h index 4d6f6e68a9..590c95e4cc 100644 --- a/call/rtp_transport_controller_send.h +++ b/call/rtp_transport_controller_send.h @@ -34,6 +34,7 @@ #include "modules/pacing/packet_router.h" #include "modules/pacing/rtp_packet_pacer.h" #include "modules/pacing/task_queue_paced_sender.h" +#include "modules/rtp_rtcp/include/rtp_rtcp_defines.h" #include "rtc_base/network_route.h" #include "rtc_base/race_checker.h" #include "rtc_base/task_queue.h" @@ -46,7 +47,7 @@ class RtcEventLog; class RtpTransportControllerSend final : public RtpTransportControllerSendInterface, - public RtcpBandwidthObserver, + public NetworkLinkRtcpObserver, public TransportFeedbackObserver, public NetworkStateEstimateObserver { public: @@ -90,7 +91,7 @@ class RtpTransportControllerSend final void OnNetworkRouteChanged(absl::string_view transport_name, const rtc::NetworkRoute& network_route) override; void OnNetworkAvailability(bool network_available) override; - RtcpBandwidthObserver* GetBandwidthObserver() override; + NetworkLinkRtcpObserver* GetRtcpObserver() override; int64_t GetPacerQueuingDelayMs() const override; absl::optional GetFirstPacketTime() const override; void EnablePeriodicAlrProbing(bool enable) override; @@ -107,15 +108,18 @@ class RtpTransportControllerSend final void IncludeOverheadInPacedSender() override; void EnsureStarted() override; - // Implements RtcpBandwidthObserver interface - void OnReceivedEstimatedBitrate(uint32_t bitrate) override; - void OnReceivedRtcpReceiverReport(const ReportBlockList& report_blocks, - int64_t rtt, - int64_t now_ms) override; + // Implements NetworkLinkRtcpObserver interface + void OnReceiverEstimatedMaxBitrate(Timestamp receive_time, + DataRate bitrate) override; + void OnReport(Timestamp receive_time, + rtc::ArrayView report_blocks) override; + void OnRttUpdate(Timestamp receive_time, TimeDelta rtt) override; + void OnTransportFeedback(Timestamp receive_time, + const rtcp::TransportFeedback& feedback) override; // Implements TransportFeedbackObserver interface void OnAddPacket(const RtpPacketSendInfo& packet_info) override; - void OnTransportFeedback(const rtcp::TransportFeedback& feedback) override; + void OnTransportFeedback(const rtcp::TransportFeedback& feedback) override {} // Implements NetworkStateEstimateObserver interface void OnRemoteNetworkEstimate(NetworkStateEstimate estimate) override; @@ -133,9 +137,6 @@ class RtpTransportControllerSend final const rtc::NetworkRoute& new_route) const; void UpdateBitrateConstraints(const BitrateConstraints& updated); void UpdateStreamsConfig() RTC_RUN_ON(sequence_checker_); - void OnReceivedRtcpReceiverReportBlocks(const ReportBlockList& report_blocks, - int64_t now_ms) - RTC_RUN_ON(sequence_checker_); void PostUpdates(NetworkControlUpdate update) RTC_RUN_ON(sequence_checker_); void UpdateControlState() RTC_RUN_ON(sequence_checker_); void UpdateCongestedState() RTC_RUN_ON(sequence_checker_); diff --git a/call/rtp_transport_controller_send_interface.h b/call/rtp_transport_controller_send_interface.h index 6ac2d84d03..349fe68039 100644 --- a/call/rtp_transport_controller_send_interface.h +++ b/call/rtp_transport_controller_send_interface.h @@ -46,7 +46,6 @@ class TargetTransferRateObserver; class Transport; class PacketRouter; class RtpVideoSenderInterface; -class RtcpBandwidthObserver; class RtpPacketSender; struct RtpSenderObservers { @@ -130,7 +129,7 @@ class RtpTransportControllerSendInterface { absl::string_view transport_name, const rtc::NetworkRoute& network_route) = 0; virtual void OnNetworkAvailability(bool network_available) = 0; - virtual RtcpBandwidthObserver* GetBandwidthObserver() = 0; + virtual NetworkLinkRtcpObserver* GetRtcpObserver() = 0; virtual int64_t GetPacerQueuingDelayMs() const = 0; virtual absl::optional GetFirstPacketTime() const = 0; virtual void EnablePeriodicAlrProbing(bool enable) = 0; diff --git a/call/rtp_video_sender.cc b/call/rtp_video_sender.cc index 27040f369e..d952fec8bc 100644 --- a/call/rtp_video_sender.cc +++ b/call/rtp_video_sender.cc @@ -191,7 +191,6 @@ std::vector CreateRtpStreamSenders( const RtpSenderObservers& observers, int rtcp_report_interval_ms, Transport* send_transport, - RtcpBandwidthObserver* bandwidth_callback, RtpTransportControllerSendInterface* transport, const std::map& suspended_ssrcs, RtcEventLog* event_log, @@ -212,7 +211,7 @@ std::vector CreateRtpStreamSenders( configuration.intra_frame_callback = observers.intra_frame_callback; configuration.rtcp_loss_notification_observer = observers.rtcp_loss_notification_observer; - configuration.bandwidth_callback = bandwidth_callback; + configuration.network_link_rtcp_observer = transport->GetRtcpObserver(); configuration.network_state_estimate_observer = transport->network_state_estimate_observer(); configuration.transport_feedback_callback = @@ -390,7 +389,6 @@ RtpVideoSender::RtpVideoSender( observers, rtcp_report_interval_ms, send_transport, - transport->GetBandwidthObserver(), transport, suspended_ssrcs, event_log, diff --git a/call/test/mock_rtp_transport_controller_send.h b/call/test/mock_rtp_transport_controller_send.h index 0c522dfff4..b24e5a59ec 100644 --- a/call/test/mock_rtp_transport_controller_send.h +++ b/call/test/mock_rtp_transport_controller_send.h @@ -79,7 +79,7 @@ class MockRtpTransportControllerSend (absl::string_view, const rtc::NetworkRoute&), (override)); MOCK_METHOD(void, OnNetworkAvailability, (bool), (override)); - MOCK_METHOD(RtcpBandwidthObserver*, GetBandwidthObserver, (), (override)); + MOCK_METHOD(NetworkLinkRtcpObserver*, GetRtcpObserver, (), (override)); MOCK_METHOD(int64_t, GetPacerQueuingDelayMs, (), (const, override)); MOCK_METHOD(absl::optional, GetFirstPacketTime,