From 2c41cbae37cac548a1133589b9d2c2e8614fa6cb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Terelius?= Date: Wed, 1 Sep 2021 16:03:26 +0000 Subject: [PATCH] Revert "Wire up non-sender RTT for audio, and implement related standardized stats." MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This reverts commit fb0dca6c055cbf9e43af665d3c437eba6f43372e. Reason for revert: Speculative revert due to failing stats test in chromium. Possibly because the chromium test expected the metrics to not be supported, and now they are. Reverting just to unblock the webrtc roll into chromium. Original change's description: > Wire up non-sender RTT for audio, and implement related standardized stats. > > The implemented stats are: > - https://www.w3.org/TR/webrtc-stats/#dom-rtcremoteoutboundrtpstreamstats-roundtriptime > - https://www.w3.org/TR/webrtc-stats/#dom-rtcremoteoutboundrtpstreamstats-totalroundtriptime > - https://www.w3.org/TR/webrtc-stats/#dom-rtcremoteoutboundrtpstreamstats-roundtriptimemeasurements > > Bug: webrtc:12951, webrtc:12714 > Change-Id: Ia362d5c4b0456140e32da79d40edc06ab9ce2a2c > Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/226956 > Commit-Queue: Ivo Creusen > Reviewed-by: Henrik Boström > Reviewed-by: Harald Alvestrand > Reviewed-by: Danil Chapovalov > Cr-Commit-Position: refs/heads/main@{#34861} # Not skipping CQ checks because original CL landed > 1 day ago. TBR=hta,hbos,minyue Bug: webrtc:12951, webrtc:12714 Change-Id: If07ad63286eea9cdde88271e61cc28f4b268b290 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/231001 Reviewed-by: Danil Chapovalov Reviewed-by: Ivo Creusen Reviewed-by: Olga Sharonova Commit-Queue: Björn Terelius Cr-Commit-Position: refs/heads/main@{#34897} --- api/stats/rtcstats_objects.h | 3 - audio/BUILD.gn | 1 - audio/audio_receive_stream.cc | 16 +- audio/audio_receive_stream.h | 1 - audio/audio_send_stream_unittest.cc | 3 +- audio/channel_receive.cc | 24 +- audio/channel_receive.h | 5 - audio/mock_voe_channel_proxy.h | 1 - audio/test/non_sender_rtt_test.cc | 58 ----- call/audio_receive_stream.h | 8 - call/audio_send_stream.cc | 3 - call/audio_send_stream.h | 1 - media/BUILD.gn | 1 - media/base/media_channel.h | 4 - media/engine/fake_webrtc_call.cc | 4 - media/engine/fake_webrtc_call.h | 1 - media/engine/webrtc_voice_engine.cc | 32 +-- media/engine/webrtc_voice_engine.h | 1 - modules/rtp_rtcp/mocks/mock_rtp_rtcp.h | 4 - modules/rtp_rtcp/source/rtcp_receiver.cc | 56 +--- modules/rtp_rtcp/source/rtcp_receiver.h | 43 +--- .../rtp_rtcp/source/rtcp_receiver_unittest.cc | 241 ------------------ modules/rtp_rtcp/source/rtp_rtcp_impl.cc | 6 - modules/rtp_rtcp/source/rtp_rtcp_impl.h | 3 - modules/rtp_rtcp/source/rtp_rtcp_impl2.cc | 11 - modules/rtp_rtcp/source/rtp_rtcp_impl2.h | 1 - .../rtp_rtcp/source/rtp_rtcp_impl_unittest.cc | 10 +- modules/rtp_rtcp/source/rtp_rtcp_interface.h | 14 +- pc/rtc_stats_collector.cc | 8 - stats/rtcstats_objects.cc | 15 +- 30 files changed, 27 insertions(+), 552 deletions(-) delete mode 100644 audio/test/non_sender_rtt_test.cc diff --git a/api/stats/rtcstats_objects.h b/api/stats/rtcstats_objects.h index 849ef80a5d..8a6327ed4c 100644 --- a/api/stats/rtcstats_objects.h +++ b/api/stats/rtcstats_objects.h @@ -593,9 +593,6 @@ class RTC_EXPORT RTCRemoteOutboundRtpStreamStats final RTCStatsMember local_id; RTCStatsMember remote_timestamp; RTCStatsMember reports_sent; - RTCStatsMember round_trip_time; - RTCStatsMember round_trip_time_measurements; - RTCStatsMember total_round_trip_time; }; // https://w3c.github.io/webrtc-stats/#dom-rtcmediasourcestats diff --git a/audio/BUILD.gn b/audio/BUILD.gn index ea6b9d2d55..4ecdcea9e7 100644 --- a/audio/BUILD.gn +++ b/audio/BUILD.gn @@ -142,7 +142,6 @@ if (rtc_include_tests) { "remix_resample_unittest.cc", "test/audio_stats_test.cc", "test/nack_test.cc", - "test/non_sender_rtt_test.cc", ] deps = [ ":audio", diff --git a/audio/audio_receive_stream.cc b/audio/audio_receive_stream.cc index 6f2444901b..f3843812ee 100644 --- a/audio/audio_receive_stream.cc +++ b/audio/audio_receive_stream.cc @@ -81,10 +81,9 @@ std::unique_ptr CreateChannelReceive( config.rtcp_send_transport, event_log, config.rtp.local_ssrc, config.rtp.remote_ssrc, config.jitter_buffer_max_packets, config.jitter_buffer_fast_accelerate, config.jitter_buffer_min_delay_ms, - config.jitter_buffer_enable_rtx_handling, config.enable_non_sender_rtt, - config.decoder_factory, config.codec_pair_id, - std::move(config.frame_decryptor), config.crypto_options, - std::move(config.frame_transformer)); + config.jitter_buffer_enable_rtx_handling, config.decoder_factory, + config.codec_pair_id, std::move(config.frame_decryptor), + config.crypto_options, std::move(config.frame_transformer)); } } // namespace @@ -243,12 +242,6 @@ void AudioReceiveStream::SetUseTransportCcAndNackHistory(bool use_transport_cc, } } -void AudioReceiveStream::SetNonSenderRttMeasurement(bool enabled) { - RTC_DCHECK_RUN_ON(&worker_thread_checker_); - config_.enable_non_sender_rtt = enabled; - channel_receive_->SetNonSenderRttMeasurement(enabled); -} - void AudioReceiveStream::SetFrameDecryptor( rtc::scoped_refptr frame_decryptor) { // TODO(bugs.webrtc.org/11993): This is called via WebRtcAudioReceiveStream, @@ -354,9 +347,6 @@ webrtc::AudioReceiveStream::Stats AudioReceiveStream::GetStats( stats.sender_reports_packets_sent = call_stats.sender_reports_packets_sent; stats.sender_reports_bytes_sent = call_stats.sender_reports_bytes_sent; stats.sender_reports_reports_count = call_stats.sender_reports_reports_count; - stats.round_trip_time = call_stats.round_trip_time; - stats.round_trip_time_measurements = call_stats.round_trip_time_measurements; - stats.total_round_trip_time = call_stats.total_round_trip_time; return stats; } diff --git a/audio/audio_receive_stream.h b/audio/audio_receive_stream.h index 444ec4586e..61ebc2719f 100644 --- a/audio/audio_receive_stream.h +++ b/audio/audio_receive_stream.h @@ -92,7 +92,6 @@ class AudioReceiveStream final : public webrtc::AudioReceiveStream, void SetDecoderMap(std::map decoder_map) override; void SetUseTransportCcAndNackHistory(bool use_transport_cc, int history_ms) override; - void SetNonSenderRttMeasurement(bool enabled) override; void SetFrameDecryptor(rtc::scoped_refptr frame_decryptor) override; void SetRtpExtensions(std::vector extensions) override; diff --git a/audio/audio_send_stream_unittest.cc b/audio/audio_send_stream_unittest.cc index 58db5254fe..db42efc373 100644 --- a/audio/audio_send_stream_unittest.cc +++ b/audio/audio_send_stream_unittest.cc @@ -387,8 +387,7 @@ TEST(AudioSendStreamTest, ConfigToString) { "min_bitrate_bps: 12000, max_bitrate_bps: 34000, has " "audio_network_adaptor_config: false, has_dscp: true, " "send_codec_spec: {nack_enabled: true, transport_cc_enabled: false, " - "enable_non_sender_rtt: false, cng_payload_type: 42, " - "red_payload_type: 43, payload_type: 103, " + "cng_payload_type: 42, red_payload_type: 43, payload_type: 103, " "format: {name: isac, clockrate_hz: 16000, num_channels: 1, " "parameters: {}}}}", config.ToString()); diff --git a/audio/channel_receive.cc b/audio/channel_receive.cc index 9c6f672c3f..b741a8c1ca 100644 --- a/audio/channel_receive.cc +++ b/audio/channel_receive.cc @@ -97,7 +97,6 @@ class ChannelReceive : public ChannelReceiveInterface, bool jitter_buffer_fast_playout, int jitter_buffer_min_delay_ms, bool jitter_buffer_enable_rtx_handling, - bool enable_non_sender_rtt, rtc::scoped_refptr decoder_factory, absl::optional codec_pair_id, rtc::scoped_refptr frame_decryptor, @@ -159,7 +158,6 @@ class ChannelReceive : public ChannelReceiveInterface, CallReceiveStatistics GetRTCPStatistics() const override; void SetNACKStatus(bool enable, int maxNumberOfPackets) override; - void SetNonSenderRttMeasurement(bool enabled) override; AudioMixer::Source::AudioFrameInfo GetAudioFrameWithInfo( int sample_rate_hz, @@ -526,7 +524,6 @@ ChannelReceive::ChannelReceive( bool jitter_buffer_fast_playout, int jitter_buffer_min_delay_ms, bool jitter_buffer_enable_rtx_handling, - bool enable_non_sender_rtt, rtc::scoped_refptr decoder_factory, absl::optional codec_pair_id, rtc::scoped_refptr frame_decryptor, @@ -576,7 +573,6 @@ ChannelReceive::ChannelReceive( configuration.event_log = event_log_; configuration.local_media_ssrc = local_ssrc; configuration.rtcp_packet_type_counter_observer = this; - configuration.non_sender_rtt_measurement = enable_non_sender_rtt; if (frame_transformer) InitFrameTransformerDelegate(std::move(frame_transformer)); @@ -860,15 +856,6 @@ CallReceiveStatistics ChannelReceive::GetRTCPStatistics() const { stats.sender_reports_reports_count = rtcp_sr_stats->reports_count; } - absl::optional non_sender_rtt_stats = - rtp_rtcp_->GetNonSenderRttStats(); - if (non_sender_rtt_stats.has_value()) { - stats.round_trip_time = non_sender_rtt_stats->round_trip_time; - stats.round_trip_time_measurements = - non_sender_rtt_stats->round_trip_time_measurements; - stats.total_round_trip_time = non_sender_rtt_stats->total_round_trip_time; - } - return stats; } @@ -885,11 +872,6 @@ void ChannelReceive::SetNACKStatus(bool enable, int max_packets) { } } -void ChannelReceive::SetNonSenderRttMeasurement(bool enabled) { - RTC_DCHECK_RUN_ON(&worker_thread_checker_); - rtp_rtcp_->SetNonSenderRttMeasurement(enabled); -} - // Called when we are missing one or more packets. int ChannelReceive::ResendPackets(const uint16_t* sequence_numbers, int length) { @@ -1128,7 +1110,6 @@ std::unique_ptr CreateChannelReceive( bool jitter_buffer_fast_playout, int jitter_buffer_min_delay_ms, bool jitter_buffer_enable_rtx_handling, - bool enable_non_sender_rtt, rtc::scoped_refptr decoder_factory, absl::optional codec_pair_id, rtc::scoped_refptr frame_decryptor, @@ -1138,9 +1119,8 @@ std::unique_ptr CreateChannelReceive( clock, neteq_factory, audio_device_module, rtcp_send_transport, rtc_event_log, local_ssrc, remote_ssrc, jitter_buffer_max_packets, jitter_buffer_fast_playout, jitter_buffer_min_delay_ms, - jitter_buffer_enable_rtx_handling, enable_non_sender_rtt, decoder_factory, - codec_pair_id, std::move(frame_decryptor), crypto_options, - std::move(frame_transformer)); + jitter_buffer_enable_rtx_handling, decoder_factory, codec_pair_id, + std::move(frame_decryptor), crypto_options, std::move(frame_transformer)); } } // namespace voe diff --git a/audio/channel_receive.h b/audio/channel_receive.h index d811e87719..deec49feaf 100644 --- a/audio/channel_receive.h +++ b/audio/channel_receive.h @@ -74,9 +74,6 @@ struct CallReceiveStatistics { uint32_t sender_reports_packets_sent = 0; uint64_t sender_reports_bytes_sent = 0; uint64_t sender_reports_reports_count = 0; - absl::optional round_trip_time; - TimeDelta total_round_trip_time = TimeDelta::Zero(); - int round_trip_time_measurements; }; namespace voe { @@ -141,7 +138,6 @@ class ChannelReceiveInterface : public RtpPacketSinkInterface { virtual CallReceiveStatistics GetRTCPStatistics() const = 0; virtual void SetNACKStatus(bool enable, int max_packets) = 0; - virtual void SetNonSenderRttMeasurement(bool enabled) = 0; virtual AudioMixer::Source::AudioFrameInfo GetAudioFrameWithInfo( int sample_rate_hz, @@ -183,7 +179,6 @@ std::unique_ptr CreateChannelReceive( bool jitter_buffer_fast_playout, int jitter_buffer_min_delay_ms, bool jitter_buffer_enable_rtx_handling, - bool enable_non_sender_rtt, rtc::scoped_refptr decoder_factory, absl::optional codec_pair_id, rtc::scoped_refptr frame_decryptor, diff --git a/audio/mock_voe_channel_proxy.h b/audio/mock_voe_channel_proxy.h index d445b51312..ea2a2ac3f0 100644 --- a/audio/mock_voe_channel_proxy.h +++ b/audio/mock_voe_channel_proxy.h @@ -30,7 +30,6 @@ namespace test { class MockChannelReceive : public voe::ChannelReceiveInterface { public: MOCK_METHOD(void, SetNACKStatus, (bool enable, int max_packets), (override)); - MOCK_METHOD(void, SetNonSenderRttMeasurement, (bool enabled), (override)); MOCK_METHOD(void, RegisterReceiverCongestionControlObjects, (PacketRouter*), diff --git a/audio/test/non_sender_rtt_test.cc b/audio/test/non_sender_rtt_test.cc deleted file mode 100644 index 5c5b15eecf..0000000000 --- a/audio/test/non_sender_rtt_test.cc +++ /dev/null @@ -1,58 +0,0 @@ -/* - * Copyright (c) 2021 The WebRTC project authors. All Rights Reserved. - * - * Use of this source code is governed by a BSD-style license - * that can be found in the LICENSE file in the root of the source - * tree. An additional intellectual property rights grant can be found - * in the file PATENTS. All contributing project authors may - * be found in the AUTHORS file in the root of the source tree. - */ - -#include "audio/test/audio_end_to_end_test.h" -#include "system_wrappers/include/sleep.h" -#include "test/gtest.h" - -namespace webrtc { -namespace test { - -using NonSenderRttTest = CallTest; - -TEST_F(NonSenderRttTest, NonSenderRttStats) { - class NonSenderRttTest : public AudioEndToEndTest { - public: - const int kTestDurationMs = 10000; - const int64_t kRttMs = 30; - - BuiltInNetworkBehaviorConfig GetNetworkPipeConfig() const override { - BuiltInNetworkBehaviorConfig pipe_config; - pipe_config.queue_delay_ms = kRttMs / 2; - return pipe_config; - } - - void ModifyAudioConfigs( - AudioSendStream::Config* send_config, - std::vector* receive_configs) override { - ASSERT_EQ(receive_configs->size(), 1U); - (*receive_configs)[0].enable_non_sender_rtt = true; - AudioEndToEndTest::ModifyAudioConfigs(send_config, receive_configs); - send_config->send_codec_spec->enable_non_sender_rtt = true; - } - - void PerformTest() override { SleepMs(kTestDurationMs); } - - void OnStreamsStopped() override { - AudioReceiveStream::Stats recv_stats = - receive_stream()->GetStats(/*get_and_clear_legacy_stats=*/true); - EXPECT_GT(recv_stats.round_trip_time_measurements, 0); - ASSERT_TRUE(recv_stats.round_trip_time.has_value()); - EXPECT_GT(recv_stats.round_trip_time->ms(), 0); - EXPECT_GE(recv_stats.total_round_trip_time.ms(), - recv_stats.round_trip_time->ms()); - } - } test; - - RunBaseTest(&test); -} - -} // namespace test -} // namespace webrtc diff --git a/call/audio_receive_stream.h b/call/audio_receive_stream.h index 1c2d11b1af..182cd49679 100644 --- a/call/audio_receive_stream.h +++ b/call/audio_receive_stream.h @@ -96,9 +96,6 @@ class AudioReceiveStream : public MediaReceiveStream { uint32_t sender_reports_packets_sent = 0; uint64_t sender_reports_bytes_sent = 0; uint64_t sender_reports_reports_count = 0; - absl::optional round_trip_time; - TimeDelta total_round_trip_time = TimeDelta::Zero(); - int round_trip_time_measurements; }; struct Config { @@ -118,9 +115,6 @@ class AudioReceiveStream : public MediaReceiveStream { NackConfig nack; } rtp; - // Receive-side RTT. - bool enable_non_sender_rtt = false; - Transport* rtcp_send_transport = nullptr; // NetEq settings. @@ -164,8 +158,6 @@ class AudioReceiveStream : public MediaReceiveStream { virtual void SetDecoderMap(std::map decoder_map) = 0; virtual void SetUseTransportCcAndNackHistory(bool use_transport_cc, int history_ms) = 0; - virtual void SetNonSenderRttMeasurement(bool enabled) = 0; - // Set/change the rtp header extensions. Must be called on the packet // delivery thread. virtual void SetRtpExtensions(std::vector extensions) = 0; diff --git a/call/audio_send_stream.cc b/call/audio_send_stream.cc index a36050a9f7..916336b929 100644 --- a/call/audio_send_stream.cc +++ b/call/audio_send_stream.cc @@ -80,8 +80,6 @@ std::string AudioSendStream::Config::SendCodecSpec::ToString() const { rtc::SimpleStringBuilder ss(buf); ss << "{nack_enabled: " << (nack_enabled ? "true" : "false"); ss << ", transport_cc_enabled: " << (transport_cc_enabled ? "true" : "false"); - ss << ", enable_non_sender_rtt: " - << (enable_non_sender_rtt ? "true" : "false"); ss << ", cng_payload_type: " << (cng_payload_type ? rtc::ToString(*cng_payload_type) : ""); ss << ", red_payload_type: " @@ -96,7 +94,6 @@ bool AudioSendStream::Config::SendCodecSpec::operator==( const AudioSendStream::Config::SendCodecSpec& rhs) const { if (nack_enabled == rhs.nack_enabled && transport_cc_enabled == rhs.transport_cc_enabled && - enable_non_sender_rtt == rhs.enable_non_sender_rtt && cng_payload_type == rhs.cng_payload_type && red_payload_type == rhs.red_payload_type && payload_type == rhs.payload_type && format == rhs.format && diff --git a/call/audio_send_stream.h b/call/audio_send_stream.h index e38a47f871..e084d4219d 100644 --- a/call/audio_send_stream.h +++ b/call/audio_send_stream.h @@ -140,7 +140,6 @@ class AudioSendStream : public AudioSender { SdpAudioFormat format; bool nack_enabled = false; bool transport_cc_enabled = false; - bool enable_non_sender_rtt = false; absl::optional cng_payload_type; absl::optional red_payload_type; // If unset, use the encoder's default target bitrate. diff --git a/media/BUILD.gn b/media/BUILD.gn index acfa09f039..99a4dcd202 100644 --- a/media/BUILD.gn +++ b/media/BUILD.gn @@ -66,7 +66,6 @@ rtc_library("rtc_media_base") { "../api/transport:stun_types", "../api/transport:webrtc_key_value_config", "../api/transport/rtp:rtp_source", - "../api/units:time_delta", "../api/video:video_bitrate_allocation", "../api/video:video_bitrate_allocator_factory", "../api/video:video_frame", diff --git a/media/base/media_channel.h b/media/base/media_channel.h index 2607a7a4aa..6467a442d6 100644 --- a/media/base/media_channel.h +++ b/media/base/media_channel.h @@ -28,7 +28,6 @@ #include "api/rtp_parameters.h" #include "api/transport/data_channel_transport_interface.h" #include "api/transport/rtp/rtp_source.h" -#include "api/units/time_delta.h" #include "api/video/video_content_type.h" #include "api/video/video_sink_interface.h" #include "api/video/video_source_interface.h" @@ -532,9 +531,6 @@ struct VoiceReceiverInfo : public MediaReceiverInfo { uint32_t sender_reports_packets_sent = 0; uint64_t sender_reports_bytes_sent = 0; uint64_t sender_reports_reports_count = 0; - absl::optional round_trip_time; - webrtc::TimeDelta total_round_trip_time = webrtc::TimeDelta::Zero(); - int round_trip_time_measurements = 0; }; struct VideoSenderInfo : public MediaSenderInfo { diff --git a/media/engine/fake_webrtc_call.cc b/media/engine/fake_webrtc_call.cc index f61aab04f1..13401514bd 100644 --- a/media/engine/fake_webrtc_call.cc +++ b/media/engine/fake_webrtc_call.cc @@ -116,10 +116,6 @@ void FakeAudioReceiveStream::SetUseTransportCcAndNackHistory( config_.rtp.nack.rtp_history_ms = history_ms; } -void FakeAudioReceiveStream::SetNonSenderRttMeasurement(bool enabled) { - config_.enable_non_sender_rtt = enabled; -} - void FakeAudioReceiveStream::SetFrameDecryptor( rtc::scoped_refptr frame_decryptor) { config_.frame_decryptor = std::move(frame_decryptor); diff --git a/media/engine/fake_webrtc_call.h b/media/engine/fake_webrtc_call.h index 5f6b8643db..aeef95477e 100644 --- a/media/engine/fake_webrtc_call.h +++ b/media/engine/fake_webrtc_call.h @@ -122,7 +122,6 @@ class FakeAudioReceiveStream final : public webrtc::AudioReceiveStream { std::map decoder_map) override; void SetUseTransportCcAndNackHistory(bool use_transport_cc, int history_ms) override; - void SetNonSenderRttMeasurement(bool enabled) override; void SetFrameDecryptor(rtc::scoped_refptr frame_decryptor) override; void SetRtpExtensions(std::vector extensions) override; diff --git a/media/engine/webrtc_voice_engine.cc b/media/engine/webrtc_voice_engine.cc index 75d1a5fe02..0cf17ed984 100644 --- a/media/engine/webrtc_voice_engine.cc +++ b/media/engine/webrtc_voice_engine.cc @@ -255,7 +255,6 @@ webrtc::AudioReceiveStream::Config BuildReceiveStreamConfig( uint32_t local_ssrc, bool use_transport_cc, bool use_nack, - bool enable_non_sender_rtt, const std::vector& stream_ids, const std::vector& extensions, webrtc::Transport* rtcp_send_transport, @@ -279,7 +278,6 @@ webrtc::AudioReceiveStream::Config BuildReceiveStreamConfig( } config.rtp.extensions = extensions; config.rtcp_send_transport = rtcp_send_transport; - config.enable_non_sender_rtt = enable_non_sender_rtt; config.decoder_factory = decoder_factory; config.decoder_map = decoder_map; config.codec_pair_id = codec_pair_id; @@ -1247,11 +1245,6 @@ class WebRtcVoiceMediaChannel::WebRtcAudioReceiveStream { use_nack ? kNackRtpHistoryMs : 0); } - void SetNonSenderRttMeasurement(bool enabled) { - RTC_DCHECK_RUN_ON(&worker_thread_checker_); - stream_->SetNonSenderRttMeasurement(enabled); - } - void SetRtpExtensions(const std::vector& extensions) { RTC_DCHECK_RUN_ON(&worker_thread_checker_); stream_->SetRtpExtensions(extensions); @@ -1722,7 +1715,6 @@ bool WebRtcVoiceMediaChannel::SetSendCodecs( } send_codec_spec->transport_cc_enabled = HasTransportCc(voice_codec); send_codec_spec->nack_enabled = HasNack(voice_codec); - send_codec_spec->enable_non_sender_rtt = HasRrtr(voice_codec); bitrate_config = GetBitrateConfigForCodec(voice_codec); break; } @@ -1798,27 +1790,16 @@ bool WebRtcVoiceMediaChannel::SetSendCodecs( // preferred send codec, and in that case reconfigure all receive streams. if (recv_transport_cc_enabled_ != send_codec_spec_->transport_cc_enabled || recv_nack_enabled_ != send_codec_spec_->nack_enabled) { - RTC_LOG(LS_INFO) << "Changing transport cc and NACK status on receive " - "streams."; + RTC_LOG(LS_INFO) << "Recreate all the receive streams because the send " + "codec has changed."; recv_transport_cc_enabled_ = send_codec_spec_->transport_cc_enabled; recv_nack_enabled_ = send_codec_spec_->nack_enabled; - enable_non_sender_rtt_ = send_codec_spec_->enable_non_sender_rtt; for (auto& kv : recv_streams_) { kv.second->SetUseTransportCc(recv_transport_cc_enabled_, recv_nack_enabled_); } } - // Check if the receive-side RTT status has changed on the preferred send - // codec, in that case reconfigure all receive streams. - if (enable_non_sender_rtt_ != send_codec_spec_->enable_non_sender_rtt) { - RTC_LOG(LS_INFO) << "Changing receive-side RTT status on receive streams."; - enable_non_sender_rtt_ = send_codec_spec_->enable_non_sender_rtt; - for (auto& kv : recv_streams_) { - kv.second->SetNonSenderRttMeasurement(enable_non_sender_rtt_); - } - } - send_codecs_ = codecs; return true; } @@ -1982,9 +1963,9 @@ bool WebRtcVoiceMediaChannel::AddRecvStream(const StreamParams& sp) { // Create a new channel for receiving audio data. auto config = BuildReceiveStreamConfig( ssrc, receiver_reports_ssrc_, recv_transport_cc_enabled_, - recv_nack_enabled_, enable_non_sender_rtt_, sp.stream_ids(), - recv_rtp_extensions_, this, engine()->decoder_factory_, decoder_map_, - codec_pair_id_, engine()->audio_jitter_buffer_max_packets_, + recv_nack_enabled_, sp.stream_ids(), recv_rtp_extensions_, this, + engine()->decoder_factory_, decoder_map_, codec_pair_id_, + engine()->audio_jitter_buffer_max_packets_, engine()->audio_jitter_buffer_fast_accelerate_, engine()->audio_jitter_buffer_min_delay_ms_, engine()->audio_jitter_buffer_enable_rtx_handling_, @@ -2443,9 +2424,6 @@ bool WebRtcVoiceMediaChannel::GetStats(VoiceMediaInfo* info, rinfo.sender_reports_packets_sent = stats.sender_reports_packets_sent; rinfo.sender_reports_bytes_sent = stats.sender_reports_bytes_sent; rinfo.sender_reports_reports_count = stats.sender_reports_reports_count; - rinfo.round_trip_time = stats.round_trip_time; - rinfo.round_trip_time_measurements = stats.round_trip_time_measurements; - rinfo.total_round_trip_time = stats.total_round_trip_time; if (recv_nack_enabled_) { rinfo.nacks_sent = stats.nacks_sent; diff --git a/media/engine/webrtc_voice_engine.h b/media/engine/webrtc_voice_engine.h index a8eb61d318..a7c2f396bb 100644 --- a/media/engine/webrtc_voice_engine.h +++ b/media/engine/webrtc_voice_engine.h @@ -277,7 +277,6 @@ class WebRtcVoiceMediaChannel final : public VoiceMediaChannel, int dtmf_payload_freq_ = -1; bool recv_transport_cc_enabled_ = false; bool recv_nack_enabled_ = false; - bool enable_non_sender_rtt_ = false; bool playout_ = false; bool send_ = false; webrtc::Call* const call_ = nullptr; diff --git a/modules/rtp_rtcp/mocks/mock_rtp_rtcp.h b/modules/rtp_rtcp/mocks/mock_rtp_rtcp.h index 05e49fb861..33c5a9bcf9 100644 --- a/modules/rtp_rtcp/mocks/mock_rtp_rtcp.h +++ b/modules/rtp_rtcp/mocks/mock_rtp_rtcp.h @@ -152,10 +152,6 @@ class MockRtpRtcpInterface : public RtpRtcpInterface { GetSenderReportStats, (), (const, override)); - MOCK_METHOD(absl::optional, - GetNonSenderRttStats, - (), - (const, override)); MOCK_METHOD(void, SetRemb, (int64_t bitrate, std::vector ssrcs), diff --git a/modules/rtp_rtcp/source/rtcp_receiver.cc b/modules/rtp_rtcp/source/rtcp_receiver.cc index e64b6932ba..32f442a7f7 100644 --- a/modules/rtp_rtcp/source/rtcp_receiver.cc +++ b/modules/rtp_rtcp/source/rtcp_receiver.cc @@ -303,15 +303,6 @@ int32_t RTCPReceiver::RTT(uint32_t remote_ssrc, return 0; } -RTCPReceiver::NonSenderRttStats RTCPReceiver::GetNonSenderRTT() const { - MutexLock lock(&rtcp_receiver_lock_); - auto it = non_sender_rtts_.find(remote_ssrc_); - if (it == non_sender_rtts_.end()) { - return {}; - } - return it->second; -} - void RTCPReceiver::SetNonSenderRttMeasurement(bool enabled) { MutexLock lock(&rtcp_receiver_lock_); xr_rrtr_status_ = enabled; @@ -444,16 +435,6 @@ bool RTCPReceiver::ParseCompoundPacket(rtc::ArrayView packet, MutexLock lock(&rtcp_receiver_lock_); CommonHeader rtcp_block; - // If a sender report is received but no DLRR, we need to reset the - // roundTripTime stat according to the standard, see - // https://www.w3.org/TR/webrtc-stats/#dom-rtcremoteoutboundrtpstreamstats-roundtriptime - struct RtcpReceivedBlock { - bool sender_report = false; - bool dlrr = false; - }; - // For each remote SSRC we store if we've received a sender report or a DLRR - // block. - flat_map received_blocks; for (const uint8_t* next_block = packet.begin(); next_block != packet.end(); next_block = rtcp_block.NextPacket()) { ptrdiff_t remaining_blocks_size = packet.end() - next_block; @@ -474,7 +455,6 @@ bool RTCPReceiver::ParseCompoundPacket(rtc::ArrayView packet, switch (rtcp_block.type()) { case rtcp::SenderReport::kPacketType: HandleSenderReport(rtcp_block, packet_information); - received_blocks[packet_information->remote_ssrc].sender_report = true; break; case rtcp::ReceiverReport::kPacketType: HandleReceiverReport(rtcp_block, packet_information); @@ -483,12 +463,7 @@ bool RTCPReceiver::ParseCompoundPacket(rtc::ArrayView packet, HandleSdes(rtcp_block, packet_information); break; case rtcp::ExtendedReports::kPacketType: - bool contains_dlrr; - uint32_t ssrc; - HandleXr(rtcp_block, packet_information, contains_dlrr, ssrc); - if (contains_dlrr) { - received_blocks[ssrc].dlrr = true; - } + HandleXr(rtcp_block, packet_information); break; case rtcp::Bye::kPacketType: HandleBye(rtcp_block); @@ -540,15 +515,6 @@ bool RTCPReceiver::ParseCompoundPacket(rtc::ArrayView packet, } } - for (const auto& rb : received_blocks) { - if (rb.second.sender_report && !rb.second.dlrr) { - auto rtt_stats = non_sender_rtts_.find(rb.first); - if (rtt_stats != non_sender_rtts_.end()) { - rtt_stats->second.Invalidate(); - } - } - } - if (packet_type_counter_observer_) { packet_type_counter_observer_->RtcpPacketTypesCounterUpdated( main_ssrc_, packet_type_counter_); @@ -866,22 +832,18 @@ void RTCPReceiver::HandleBye(const CommonHeader& rtcp_block) { } void RTCPReceiver::HandleXr(const CommonHeader& rtcp_block, - PacketInformation* packet_information, - bool& contains_dlrr, - uint32_t& ssrc) { + PacketInformation* packet_information) { rtcp::ExtendedReports xr; if (!xr.Parse(rtcp_block)) { ++num_skipped_packets_; return; } - ssrc = xr.sender_ssrc(); - contains_dlrr = !xr.dlrr().sub_blocks().empty(); if (xr.rrtr()) HandleXrReceiveReferenceTime(xr.sender_ssrc(), *xr.rrtr()); for (const rtcp::ReceiveTimeInfo& time_info : xr.dlrr().sub_blocks()) - HandleXrDlrrReportBlock(xr.sender_ssrc(), time_info); + HandleXrDlrrReportBlock(time_info); if (xr.target_bitrate()) { HandleXrTargetBitrate(xr.sender_ssrc(), *xr.target_bitrate(), @@ -910,8 +872,7 @@ void RTCPReceiver::HandleXrReceiveReferenceTime(uint32_t sender_ssrc, } } -void RTCPReceiver::HandleXrDlrrReportBlock(uint32_t sender_ssrc, - const rtcp::ReceiveTimeInfo& rti) { +void RTCPReceiver::HandleXrDlrrReportBlock(const rtcp::ReceiveTimeInfo& rti) { if (!registered_ssrcs_.contains(rti.ssrc)) // Not to us. return; @@ -923,21 +884,14 @@ void RTCPReceiver::HandleXrDlrrReportBlock(uint32_t sender_ssrc, uint32_t send_time_ntp = rti.last_rr; // RFC3611, section 4.5, LRR field discription states: // If no such block has been received, the field is set to zero. - if (send_time_ntp == 0) { - auto rtt_stats = non_sender_rtts_.find(sender_ssrc); - if (rtt_stats != non_sender_rtts_.end()) { - rtt_stats->second.Invalidate(); - } + if (send_time_ntp == 0) return; - } uint32_t delay_ntp = rti.delay_since_last_rr; uint32_t now_ntp = CompactNtp(clock_->CurrentNtpTime()); uint32_t rtt_ntp = now_ntp - delay_ntp - send_time_ntp; xr_rr_rtt_ms_ = CompactNtpRttToMs(rtt_ntp); - - non_sender_rtts_[sender_ssrc].Update(TimeDelta::Millis(xr_rr_rtt_ms_)); } void RTCPReceiver::HandleXrTargetBitrate( diff --git a/modules/rtp_rtcp/source/rtcp_receiver.h b/modules/rtp_rtcp/source/rtcp_receiver.h index f45b783701..206b63ae8f 100644 --- a/modules/rtp_rtcp/source/rtcp_receiver.h +++ b/modules/rtp_rtcp/source/rtcp_receiver.h @@ -16,10 +16,8 @@ #include #include -#include "absl/types/optional.h" #include "api/array_view.h" #include "api/sequence_checker.h" -#include "api/units/time_delta.h" #include "modules/rtp_rtcp/include/report_block_data.h" #include "modules/rtp_rtcp/include/rtcp_statistics.h" #include "modules/rtp_rtcp/include/rtp_rtcp_defines.h" @@ -60,35 +58,6 @@ class RTCPReceiver final { protected: virtual ~ModuleRtpRtcp() = default; }; - // Standardized stats derived from the non-sender RTT. - class NonSenderRttStats { - public: - NonSenderRttStats() = default; - NonSenderRttStats(const NonSenderRttStats&) = default; - NonSenderRttStats& operator=(const NonSenderRttStats&) = default; - ~NonSenderRttStats() = default; - void Update(TimeDelta non_sender_rtt_seconds) { - round_trip_time_ = non_sender_rtt_seconds; - total_round_trip_time_ += non_sender_rtt_seconds; - round_trip_time_measurements_++; - } - void Invalidate() { round_trip_time_.reset(); } - // https://www.w3.org/TR/webrtc-stats/#dom-rtcremoteoutboundrtpstreamstats-roundtriptime - absl::optional round_trip_time() const { - return round_trip_time_; - } - // https://www.w3.org/TR/webrtc-stats/#dom-rtcremoteoutboundrtpstreamstats-totalroundtriptime - TimeDelta total_round_trip_time() const { return total_round_trip_time_; } - // https://www.w3.org/TR/webrtc-stats/#dom-rtcremoteoutboundrtpstreamstats-roundtriptimemeasurements - int round_trip_time_measurements() const { - return round_trip_time_measurements_; - } - - private: - absl::optional round_trip_time_; - TimeDelta total_round_trip_time_ = TimeDelta::Zero(); - int round_trip_time_measurements_ = 0; - }; RTCPReceiver(const RtpRtcpInterface::Configuration& config, ModuleRtpRtcp* owner); @@ -139,9 +108,6 @@ class RTCPReceiver final { int64_t* min_rtt_ms, int64_t* max_rtt_ms) const; - // Returns non-sender RTT metrics for the remote SSRC. - NonSenderRttStats GetNonSenderRTT() const; - void SetNonSenderRttMeasurement(bool enabled); bool GetAndResetXrRrRtt(int64_t* rtt_ms); @@ -311,16 +277,14 @@ class RTCPReceiver final { RTC_EXCLUSIVE_LOCKS_REQUIRED(rtcp_receiver_lock_); void HandleXr(const rtcp::CommonHeader& rtcp_block, - PacketInformation* packet_information, - bool& contains_dlrr, - uint32_t& ssrc) + PacketInformation* packet_information) RTC_EXCLUSIVE_LOCKS_REQUIRED(rtcp_receiver_lock_); void HandleXrReceiveReferenceTime(uint32_t sender_ssrc, const rtcp::Rrtr& rrtr) RTC_EXCLUSIVE_LOCKS_REQUIRED(rtcp_receiver_lock_); - void HandleXrDlrrReportBlock(uint32_t ssrc, const rtcp::ReceiveTimeInfo& rti) + void HandleXrDlrrReportBlock(const rtcp::ReceiveTimeInfo& rti) RTC_EXCLUSIVE_LOCKS_REQUIRED(rtcp_receiver_lock_); void HandleXrTargetBitrate(uint32_t ssrc, @@ -418,9 +382,6 @@ class RTCPReceiver final { // Round-Trip Time per remote sender ssrc. flat_map rtts_ RTC_GUARDED_BY(rtcp_receiver_lock_); - // Non-sender Round-trip time per remote ssrc. - flat_map non_sender_rtts_ - RTC_GUARDED_BY(rtcp_receiver_lock_); // Report blocks per local source ssrc. flat_map received_report_blocks_ diff --git a/modules/rtp_rtcp/source/rtcp_receiver_unittest.cc b/modules/rtp_rtcp/source/rtcp_receiver_unittest.cc index fa7d569012..585d6980e6 100644 --- a/modules/rtp_rtcp/source/rtcp_receiver_unittest.cc +++ b/modules/rtp_rtcp/source/rtcp_receiver_unittest.cc @@ -802,11 +802,6 @@ TEST(RtcpReceiverTest, ExtendedReportsDlrrPacketNotToUsIgnored) { int64_t rtt_ms = 0; EXPECT_FALSE(receiver.GetAndResetXrRrRtt(&rtt_ms)); - RTCPReceiver::NonSenderRttStats non_sender_rtt_stats = - receiver.GetNonSenderRTT(); - EXPECT_FALSE(non_sender_rtt_stats.round_trip_time().has_value()); - EXPECT_TRUE(non_sender_rtt_stats.total_round_trip_time().IsZero()); - EXPECT_EQ(non_sender_rtt_stats.round_trip_time_measurements(), 0); } TEST(RtcpReceiverTest, InjectExtendedReportsDlrrPacketWithSubBlock) { @@ -831,11 +826,6 @@ TEST(RtcpReceiverTest, InjectExtendedReportsDlrrPacketWithSubBlock) { EXPECT_TRUE(receiver.GetAndResetXrRrRtt(&rtt_ms)); uint32_t rtt_ntp = compact_ntp_now - kDelay - kLastRR; EXPECT_NEAR(CompactNtpRttToMs(rtt_ntp), rtt_ms, 1); - RTCPReceiver::NonSenderRttStats non_sender_rtt_stats = - receiver.GetNonSenderRTT(); - EXPECT_GT(non_sender_rtt_stats.round_trip_time(), TimeDelta::Zero()); - EXPECT_FALSE(non_sender_rtt_stats.total_round_trip_time().IsZero()); - EXPECT_GT(non_sender_rtt_stats.round_trip_time_measurements(), 0); } TEST(RtcpReceiverTest, InjectExtendedReportsDlrrPacketWithMultipleSubBlocks) { @@ -861,11 +851,6 @@ TEST(RtcpReceiverTest, InjectExtendedReportsDlrrPacketWithMultipleSubBlocks) { EXPECT_TRUE(receiver.GetAndResetXrRrRtt(&rtt_ms)); uint32_t rtt_ntp = compact_ntp_now - kDelay - kLastRR; EXPECT_NEAR(CompactNtpRttToMs(rtt_ntp), rtt_ms, 1); - RTCPReceiver::NonSenderRttStats non_sender_rtt_stats = - receiver.GetNonSenderRTT(); - EXPECT_GT(non_sender_rtt_stats.round_trip_time(), TimeDelta::Zero()); - EXPECT_FALSE(non_sender_rtt_stats.total_round_trip_time().IsZero()); - EXPECT_GT(non_sender_rtt_stats.round_trip_time_measurements(), 0); } TEST(RtcpReceiverTest, InjectExtendedReportsPacketWithMultipleReportBlocks) { @@ -916,11 +901,6 @@ TEST(RtcpReceiverTest, InjectExtendedReportsPacketWithUnknownReportBlock) { // Validate Dlrr report wasn't processed. int64_t rtt_ms = 0; EXPECT_FALSE(receiver.GetAndResetXrRrRtt(&rtt_ms)); - RTCPReceiver::NonSenderRttStats non_sender_rtt_stats = - receiver.GetNonSenderRTT(); - EXPECT_FALSE(non_sender_rtt_stats.round_trip_time().has_value()); - EXPECT_TRUE(non_sender_rtt_stats.total_round_trip_time().IsZero()); - EXPECT_EQ(non_sender_rtt_stats.round_trip_time_measurements(), 0); } TEST(RtcpReceiverTest, TestExtendedReportsRrRttInitiallyFalse) { @@ -932,11 +912,6 @@ TEST(RtcpReceiverTest, TestExtendedReportsRrRttInitiallyFalse) { int64_t rtt_ms; EXPECT_FALSE(receiver.GetAndResetXrRrRtt(&rtt_ms)); - RTCPReceiver::NonSenderRttStats non_sender_rtt_stats = - receiver.GetNonSenderRTT(); - EXPECT_FALSE(non_sender_rtt_stats.round_trip_time().has_value()); - EXPECT_TRUE(non_sender_rtt_stats.total_round_trip_time().IsZero()); - EXPECT_EQ(non_sender_rtt_stats.round_trip_time_measurements(), 0); } TEST(RtcpReceiverTest, RttCalculatedAfterExtendedReportsDlrr) { @@ -963,12 +938,6 @@ TEST(RtcpReceiverTest, RttCalculatedAfterExtendedReportsDlrr) { int64_t rtt_ms = 0; EXPECT_TRUE(receiver.GetAndResetXrRrRtt(&rtt_ms)); EXPECT_NEAR(kRttMs, rtt_ms, 1); - RTCPReceiver::NonSenderRttStats non_sender_rtt_stats = - receiver.GetNonSenderRTT(); - EXPECT_TRUE(non_sender_rtt_stats.round_trip_time().has_value()); - EXPECT_FALSE(non_sender_rtt_stats.round_trip_time().value().IsZero()); - EXPECT_FALSE(non_sender_rtt_stats.total_round_trip_time().IsZero()); - EXPECT_GT(non_sender_rtt_stats.round_trip_time_measurements(), 0); } // Same test as above but enables receive-side RTT using the setter instead of @@ -998,12 +967,6 @@ TEST(RtcpReceiverTest, SetterEnablesReceiverRtt) { int64_t rtt_ms = 0; EXPECT_TRUE(receiver.GetAndResetXrRrRtt(&rtt_ms)); EXPECT_NEAR(rtt_ms, kRttMs, 1); - RTCPReceiver::NonSenderRttStats non_sender_rtt_stats = - receiver.GetNonSenderRTT(); - EXPECT_TRUE(non_sender_rtt_stats.round_trip_time().has_value()); - EXPECT_FALSE(non_sender_rtt_stats.round_trip_time().value().IsZero()); - EXPECT_FALSE(non_sender_rtt_stats.total_round_trip_time().IsZero()); - EXPECT_GT(non_sender_rtt_stats.round_trip_time_measurements(), 0); } // Same test as above but disables receive-side RTT using the setter instead of @@ -1033,11 +996,6 @@ TEST(RtcpReceiverTest, DoesntCalculateRttOnReceivedDlrr) { // We expect that no RTT is available (because receive-side RTT was disabled). int64_t rtt_ms = 0; EXPECT_FALSE(receiver.GetAndResetXrRrRtt(&rtt_ms)); - RTCPReceiver::NonSenderRttStats non_sender_rtt_stats = - receiver.GetNonSenderRTT(); - EXPECT_FALSE(non_sender_rtt_stats.round_trip_time().has_value()); - EXPECT_TRUE(non_sender_rtt_stats.total_round_trip_time().IsZero()); - EXPECT_EQ(non_sender_rtt_stats.round_trip_time_measurements(), 0); } TEST(RtcpReceiverTest, XrDlrrCalculatesNegativeRttAsOne) { @@ -1064,205 +1022,6 @@ TEST(RtcpReceiverTest, XrDlrrCalculatesNegativeRttAsOne) { int64_t rtt_ms = 0; EXPECT_TRUE(receiver.GetAndResetXrRrRtt(&rtt_ms)); EXPECT_EQ(1, rtt_ms); - RTCPReceiver::NonSenderRttStats non_sender_rtt_stats = - receiver.GetNonSenderRTT(); - EXPECT_TRUE(non_sender_rtt_stats.round_trip_time().has_value()); - EXPECT_FALSE(non_sender_rtt_stats.round_trip_time().value().IsZero()); - EXPECT_FALSE(non_sender_rtt_stats.total_round_trip_time().IsZero()); - EXPECT_GT(non_sender_rtt_stats.round_trip_time_measurements(), 0); -} - -// Test receiver RTT stats with multiple measurements. -TEST(RtcpReceiverTest, ReceiverRttWithMultipleMeasurements) { - ReceiverMocks mocks; - auto config = DefaultConfiguration(&mocks); - config.non_sender_rtt_measurement = true; - RTCPReceiver receiver(config, &mocks.rtp_rtcp_impl); - receiver.SetRemoteSSRC(kSenderSsrc); - - Random rand(0x0123456789abcdef); - const int64_t kRttMs = rand.Rand(1, 9 * 3600 * 1000); - const uint32_t kDelayNtp = rand.Rand(0, 0x7fffffff); - const int64_t kDelayMs = CompactNtpRttToMs(kDelayNtp); - NtpTime now = mocks.clock.CurrentNtpTime(); - uint32_t sent_ntp = CompactNtp(now); - mocks.clock.AdvanceTimeMilliseconds(kRttMs + kDelayMs); - - rtcp::ExtendedReports xr; - xr.SetSenderSsrc(kSenderSsrc); - xr.AddDlrrItem(ReceiveTimeInfo(kReceiverMainSsrc, sent_ntp, kDelayNtp)); - - receiver.IncomingPacket(xr.Build()); - - // Check that the non-sender RTT stats are valid and based on a single - // measurement. - RTCPReceiver::NonSenderRttStats non_sender_rtt_stats = - receiver.GetNonSenderRTT(); - EXPECT_TRUE(non_sender_rtt_stats.round_trip_time().has_value()); - EXPECT_NEAR(non_sender_rtt_stats.round_trip_time()->ms(), kRttMs, 1); - EXPECT_EQ(non_sender_rtt_stats.round_trip_time_measurements(), 1); - EXPECT_EQ(non_sender_rtt_stats.total_round_trip_time().ms(), - non_sender_rtt_stats.round_trip_time()->ms()); - - // Generate another XR report with the same RTT and delay. - NtpTime now2 = mocks.clock.CurrentNtpTime(); - uint32_t sent_ntp2 = CompactNtp(now2); - mocks.clock.AdvanceTimeMilliseconds(kRttMs + kDelayMs); - - rtcp::ExtendedReports xr2; - xr2.SetSenderSsrc(kSenderSsrc); - xr2.AddDlrrItem(ReceiveTimeInfo(kReceiverMainSsrc, sent_ntp2, kDelayNtp)); - - receiver.IncomingPacket(xr2.Build()); - - // Check that the non-sender RTT stats are based on 2 measurements, and that - // the values are as expected. - non_sender_rtt_stats = receiver.GetNonSenderRTT(); - EXPECT_TRUE(non_sender_rtt_stats.round_trip_time().has_value()); - EXPECT_NEAR(non_sender_rtt_stats.round_trip_time()->ms(), kRttMs, 1); - EXPECT_EQ(non_sender_rtt_stats.round_trip_time_measurements(), 2); - EXPECT_NEAR(non_sender_rtt_stats.total_round_trip_time().ms(), 2 * kRttMs, 2); -} - -// Test that the receiver RTT stat resets when receiving a SR without XR. This -// behavior is described in the standard, see -// https://www.w3.org/TR/webrtc-stats/#dom-rtcremoteoutboundrtpstreamstats-roundtriptime. -TEST(RtcpReceiverTest, ReceiverRttResetOnSrWithoutXr) { - ReceiverMocks mocks; - auto config = DefaultConfiguration(&mocks); - config.non_sender_rtt_measurement = true; - RTCPReceiver receiver(config, &mocks.rtp_rtcp_impl); - receiver.SetRemoteSSRC(kSenderSsrc); - - Random rand(0x0123456789abcdef); - const int64_t kRttMs = rand.Rand(1, 9 * 3600 * 1000); - const uint32_t kDelayNtp = rand.Rand(0, 0x7fffffff); - const int64_t kDelayMs = CompactNtpRttToMs(kDelayNtp); - NtpTime now = mocks.clock.CurrentNtpTime(); - uint32_t sent_ntp = CompactNtp(now); - mocks.clock.AdvanceTimeMilliseconds(kRttMs + kDelayMs); - - rtcp::ExtendedReports xr; - xr.SetSenderSsrc(kSenderSsrc); - xr.AddDlrrItem(ReceiveTimeInfo(kReceiverMainSsrc, sent_ntp, kDelayNtp)); - - receiver.IncomingPacket(xr.Build()); - - RTCPReceiver::NonSenderRttStats non_sender_rtt_stats = - receiver.GetNonSenderRTT(); - EXPECT_TRUE(non_sender_rtt_stats.round_trip_time().has_value()); - EXPECT_NEAR(non_sender_rtt_stats.round_trip_time()->ms(), kRttMs, 1); - - // Generate a SR without XR. - rtcp::ReportBlock rb; - rb.SetMediaSsrc(kReceiverMainSsrc); - rtcp::SenderReport sr; - sr.SetSenderSsrc(kSenderSsrc); - sr.AddReportBlock(rb); - EXPECT_CALL(mocks.rtp_rtcp_impl, OnReceivedRtcpReportBlocks); - EXPECT_CALL(mocks.bandwidth_observer, OnReceivedRtcpReceiverReport); - - receiver.IncomingPacket(sr.Build()); - - // Check that the non-sender RTT stat is not set. - non_sender_rtt_stats = receiver.GetNonSenderRTT(); - EXPECT_FALSE(non_sender_rtt_stats.round_trip_time().has_value()); -} - -// Test that the receiver RTT stat resets when receiving a DLRR with a timestamp -// of zero. This behavior is described in the standard, see -// https://www.w3.org/TR/webrtc-stats/#dom-rtcremoteoutboundrtpstreamstats-roundtriptime. -TEST(RtcpReceiverTest, ReceiverRttResetOnDlrrWithZeroTimestamp) { - ReceiverMocks mocks; - auto config = DefaultConfiguration(&mocks); - config.non_sender_rtt_measurement = true; - RTCPReceiver receiver(config, &mocks.rtp_rtcp_impl); - receiver.SetRemoteSSRC(kSenderSsrc); - - Random rand(0x0123456789abcdef); - const int64_t kRttMs = rand.Rand(1, 9 * 3600 * 1000); - const uint32_t kDelayNtp = rand.Rand(0, 0x7fffffff); - const int64_t kDelayMs = CompactNtpRttToMs(kDelayNtp); - NtpTime now = mocks.clock.CurrentNtpTime(); - uint32_t sent_ntp = CompactNtp(now); - mocks.clock.AdvanceTimeMilliseconds(kRttMs + kDelayMs); - - rtcp::ExtendedReports xr; - xr.SetSenderSsrc(kSenderSsrc); - xr.AddDlrrItem(ReceiveTimeInfo(kReceiverMainSsrc, sent_ntp, kDelayNtp)); - - receiver.IncomingPacket(xr.Build()); - - RTCPReceiver::NonSenderRttStats non_sender_rtt_stats = - receiver.GetNonSenderRTT(); - EXPECT_TRUE(non_sender_rtt_stats.round_trip_time().has_value()); - EXPECT_NEAR(non_sender_rtt_stats.round_trip_time()->ms(), kRttMs, 1); - - // Generate an XR+DLRR with zero timestamp. - rtcp::ExtendedReports xr2; - xr2.SetSenderSsrc(kSenderSsrc); - xr2.AddDlrrItem(ReceiveTimeInfo(kReceiverMainSsrc, 0, kDelayMs)); - - receiver.IncomingPacket(xr2.Build()); - - // Check that the non-sender RTT stat is not set. - non_sender_rtt_stats = receiver.GetNonSenderRTT(); - EXPECT_FALSE(non_sender_rtt_stats.round_trip_time().has_value()); -} - -// Check that the receiver RTT works correctly when the remote SSRC changes. -TEST(RtcpReceiverTest, ReceiverRttWithMultipleRemoteSsrcs) { - ReceiverMocks mocks; - auto config = DefaultConfiguration(&mocks); - config.non_sender_rtt_measurement = false; - RTCPReceiver receiver(config, &mocks.rtp_rtcp_impl); - receiver.SetRemoteSSRC(kSenderSsrc); - receiver.SetNonSenderRttMeasurement(true); - - Random rand(0x0123456789abcdef); - const int64_t kRttMs = rand.Rand(1, 9 * 3600 * 1000); - const uint32_t kDelayNtp = rand.Rand(0, 0x7fffffff); - const int64_t kDelayMs = CompactNtpRttToMs(kDelayNtp); - NtpTime now = mocks.clock.CurrentNtpTime(); - uint32_t sent_ntp = CompactNtp(now); - mocks.clock.AdvanceTimeMilliseconds(kRttMs + kDelayMs); - - rtcp::ExtendedReports xr; - xr.SetSenderSsrc(kSenderSsrc); - xr.AddDlrrItem(ReceiveTimeInfo(kReceiverMainSsrc, sent_ntp, kDelayNtp)); - - receiver.IncomingPacket(xr.Build()); - - // Generate an XR report for another SSRC. - const int64_t kRttMs2 = rand.Rand(1, 9 * 3600 * 1000); - const uint32_t kDelayNtp2 = rand.Rand(0, 0x7fffffff); - const int64_t kDelayMs2 = CompactNtpRttToMs(kDelayNtp2); - NtpTime now2 = mocks.clock.CurrentNtpTime(); - uint32_t sent_ntp2 = CompactNtp(now2); - mocks.clock.AdvanceTimeMilliseconds(kRttMs2 + kDelayMs2); - - rtcp::ExtendedReports xr2; - xr2.SetSenderSsrc(kSenderSsrc + 1); - xr2.AddDlrrItem(ReceiveTimeInfo(kReceiverMainSsrc, sent_ntp2, kDelayNtp2)); - - receiver.IncomingPacket(xr2.Build()); - - // Check that the non-sender RTT stats match the first XR. - RTCPReceiver::NonSenderRttStats non_sender_rtt_stats = - receiver.GetNonSenderRTT(); - EXPECT_TRUE(non_sender_rtt_stats.round_trip_time().has_value()); - EXPECT_NEAR(non_sender_rtt_stats.round_trip_time()->ms(), kRttMs, 1); - EXPECT_FALSE(non_sender_rtt_stats.total_round_trip_time().IsZero()); - EXPECT_GT(non_sender_rtt_stats.round_trip_time_measurements(), 0); - - // Change the remote SSRC and check that the stats match the second XR. - receiver.SetRemoteSSRC(kSenderSsrc + 1); - RTCPReceiver::NonSenderRttStats non_sender_rtt_stats2 = - receiver.GetNonSenderRTT(); - EXPECT_TRUE(non_sender_rtt_stats2.round_trip_time().has_value()); - EXPECT_NEAR(non_sender_rtt_stats2.round_trip_time()->ms(), kRttMs2, 1); - EXPECT_FALSE(non_sender_rtt_stats2.total_round_trip_time().IsZero()); - EXPECT_GT(non_sender_rtt_stats2.round_trip_time_measurements(), 0); } TEST(RtcpReceiverTest, ConsumeReceivedXrReferenceTimeInfoInitiallyEmpty) { diff --git a/modules/rtp_rtcp/source/rtp_rtcp_impl.cc b/modules/rtp_rtcp/source/rtp_rtcp_impl.cc index 3dda8c6710..a9bd671a34 100644 --- a/modules/rtp_rtcp/source/rtp_rtcp_impl.cc +++ b/modules/rtp_rtcp/source/rtp_rtcp_impl.cc @@ -603,12 +603,6 @@ ModuleRtpRtcpImpl::GetSenderReportStats() const { return absl::nullopt; } -absl::optional -ModuleRtpRtcpImpl::GetNonSenderRttStats() const { - // This is not implemented for this legacy class. - return absl::nullopt; -} - // (REMB) Receiver Estimated Max Bitrate. void ModuleRtpRtcpImpl::SetRemb(int64_t bitrate_bps, std::vector ssrcs) { diff --git a/modules/rtp_rtcp/source/rtp_rtcp_impl.h b/modules/rtp_rtcp/source/rtp_rtcp_impl.h index 655691c9e1..2ffe013483 100644 --- a/modules/rtp_rtcp/source/rtp_rtcp_impl.h +++ b/modules/rtp_rtcp/source/rtp_rtcp_impl.h @@ -200,9 +200,6 @@ class ModuleRtpRtcpImpl : public RtpRtcp, public RTCPReceiver::ModuleRtpRtcp { // which is the SSRC of the corresponding outbound RTP stream, is unique. std::vector GetLatestReportBlockData() const override; absl::optional GetSenderReportStats() const override; - // Round trip time statistics computed from the XR block contained in the last - // report. - absl::optional GetNonSenderRttStats() const override; // (REMB) Receiver Estimated Max Bitrate. void SetRemb(int64_t bitrate_bps, std::vector ssrcs) override; diff --git a/modules/rtp_rtcp/source/rtp_rtcp_impl2.cc b/modules/rtp_rtcp/source/rtp_rtcp_impl2.cc index 73ae138085..88de8db387 100644 --- a/modules/rtp_rtcp/source/rtp_rtcp_impl2.cc +++ b/modules/rtp_rtcp/source/rtp_rtcp_impl2.cc @@ -552,17 +552,6 @@ ModuleRtpRtcpImpl2::GetSenderReportStats() const { return absl::nullopt; } -absl::optional -ModuleRtpRtcpImpl2::GetNonSenderRttStats() const { - RTCPReceiver::NonSenderRttStats non_sender_rtt_stats = - rtcp_receiver_.GetNonSenderRTT(); - return {{ - non_sender_rtt_stats.round_trip_time(), - non_sender_rtt_stats.total_round_trip_time(), - non_sender_rtt_stats.round_trip_time_measurements(), - }}; -} - // (REMB) Receiver Estimated Max Bitrate. void ModuleRtpRtcpImpl2::SetRemb(int64_t bitrate_bps, std::vector ssrcs) { diff --git a/modules/rtp_rtcp/source/rtp_rtcp_impl2.h b/modules/rtp_rtcp/source/rtp_rtcp_impl2.h index 57a3fd1391..a5038956ed 100644 --- a/modules/rtp_rtcp/source/rtp_rtcp_impl2.h +++ b/modules/rtp_rtcp/source/rtp_rtcp_impl2.h @@ -213,7 +213,6 @@ class ModuleRtpRtcpImpl2 final : public RtpRtcpInterface, // which is the SSRC of the corresponding outbound RTP stream, is unique. std::vector GetLatestReportBlockData() const override; absl::optional GetSenderReportStats() const override; - absl::optional GetNonSenderRttStats() const override; // (REMB) Receiver Estimated Max Bitrate. void SetRemb(int64_t bitrate_bps, std::vector ssrcs) override; diff --git a/modules/rtp_rtcp/source/rtp_rtcp_impl_unittest.cc b/modules/rtp_rtcp/source/rtp_rtcp_impl_unittest.cc index 57b564acf4..70c9a1cf40 100644 --- a/modules/rtp_rtcp/source/rtp_rtcp_impl_unittest.cc +++ b/modules/rtp_rtcp/source/rtp_rtcp_impl_unittest.cc @@ -616,12 +616,12 @@ TEST_F(RtpRtcpImplTest, StoresPacketInfoForSentPackets) { /*is_last=*/1))); } -// Checks that the remote sender stats are not available if no RTCP SR was sent. +// Checks that the sender report stats are not available if no RTCP SR was sent. TEST_F(RtpRtcpImplTest, SenderReportStatsNotAvailable) { EXPECT_THAT(receiver_.impl_->GetSenderReportStats(), Eq(absl::nullopt)); } -// Checks that the remote sender stats are available if an RTCP SR was sent. +// Checks that the sender report stats are available if an RTCP SR was sent. TEST_F(RtpRtcpImplTest, SenderReportStatsAvailable) { // Send a frame in order to send an SR. SendFrame(&sender_, sender_video_.get(), kBaseLayerTid); @@ -630,7 +630,7 @@ TEST_F(RtpRtcpImplTest, SenderReportStatsAvailable) { EXPECT_THAT(receiver_.impl_->GetSenderReportStats(), Not(Eq(absl::nullopt))); } -// Checks that the remote sender stats are not available if an RTCP SR with an +// Checks that the sender report stats are not available if an RTCP SR with an // unexpected SSRC is received. TEST_F(RtpRtcpImplTest, SenderReportStatsNotUpdatedWithUnexpectedSsrc) { constexpr uint32_t kUnexpectedSenderSsrc = 0x87654321; @@ -670,7 +670,7 @@ TEST_F(RtpRtcpImplTest, SenderReportStatsCheckStatsFromLastReport) { Field(&SenderReportStats::bytes_sent, Eq(kOctetCount))))); } -// Checks that the remote sender stats count equals the number of sent RTCP SRs. +// Checks that the sender report stats count equals the number of sent RTCP SRs. TEST_F(RtpRtcpImplTest, SenderReportStatsCount) { using SenderReportStats = RtpRtcpInterface::SenderReportStats; // Send a frame in order to send an SR. @@ -685,7 +685,7 @@ TEST_F(RtpRtcpImplTest, SenderReportStatsCount) { Optional(Field(&SenderReportStats::reports_count, Eq(2u)))); } -// Checks that the remote sender stats include a valid arrival time if an RTCP +// Checks that the sender report stats include a valid arrival time if an RTCP // SR was sent. TEST_F(RtpRtcpImplTest, SenderReportStatsArrivalTimestampSet) { // Send a frame in order to send an SR. diff --git a/modules/rtp_rtcp/source/rtp_rtcp_interface.h b/modules/rtp_rtcp/source/rtp_rtcp_interface.h index 13ebb0ab63..df5593cc54 100644 --- a/modules/rtp_rtcp/source/rtp_rtcp_interface.h +++ b/modules/rtp_rtcp/source/rtp_rtcp_interface.h @@ -153,7 +153,7 @@ class RtpRtcpInterface : public RtcpFeedbackSenderInterface { // Stats for RTCP sender reports (SR) for a specific SSRC. // Refer to https://tools.ietf.org/html/rfc3550#section-6.4.1. struct SenderReportStats { - // Arrival NTP timestamp for the last received RTCP SR. + // Arrival NPT timestamp for the last received RTCP SR. NtpTime last_arrival_timestamp; // Received (a.k.a., remote) NTP timestamp for the last received RTCP SR. NtpTime last_remote_timestamp; @@ -170,16 +170,6 @@ class RtpRtcpInterface : public RtcpFeedbackSenderInterface { // https://www.w3.org/TR/webrtc-stats/#dom-rtcremoteoutboundrtpstreamstats-reportssent. uint64_t reports_count; }; - // Stats about the non-sender SSRC, based on RTCP extended reports (XR). - // Refer to https://datatracker.ietf.org/doc/html/rfc3611#section-2. - struct NonSenderRttStats { - // https://www.w3.org/TR/webrtc-stats/#dom-rtcremoteoutboundrtpstreamstats-roundtriptime - absl::optional round_trip_time; - // https://www.w3.org/TR/webrtc-stats/#dom-rtcremoteoutboundrtpstreamstats-totalroundtriptime - TimeDelta total_round_trip_time = TimeDelta::Zero(); - // https://www.w3.org/TR/webrtc-stats/#dom-rtcremoteoutboundrtpstreamstats-roundtriptimemeasurements - int round_trip_time_measurements = 0; - }; // ************************************************************************** // Receiver functions @@ -413,8 +403,6 @@ class RtpRtcpInterface : public RtcpFeedbackSenderInterface { virtual std::vector GetLatestReportBlockData() const = 0; // Returns stats based on the received RTCP SRs. virtual absl::optional GetSenderReportStats() const = 0; - // Returns non-sender RTT stats, based on DLRR. - virtual absl::optional GetNonSenderRttStats() const = 0; // (REMB) Receiver Estimated Max Bitrate. // Schedules sending REMB on next and following sender/receiver reports. diff --git a/pc/rtc_stats_collector.cc b/pc/rtc_stats_collector.cc index 1f8b28df62..c2b453ef00 100644 --- a/pc/rtc_stats_collector.cc +++ b/pc/rtc_stats_collector.cc @@ -436,14 +436,6 @@ CreateRemoteOutboundAudioStreamStats( stats->remote_timestamp = static_cast( voice_receiver_info.last_sender_report_remote_timestamp_ms.value()); stats->reports_sent = voice_receiver_info.sender_reports_reports_count; - if (voice_receiver_info.round_trip_time) { - stats->round_trip_time = - voice_receiver_info.round_trip_time->seconds(); - } - stats->round_trip_time_measurements = - voice_receiver_info.round_trip_time_measurements; - stats->total_round_trip_time = - voice_receiver_info.total_round_trip_time.seconds(); return stats; } diff --git a/stats/rtcstats_objects.cc b/stats/rtcstats_objects.cc index 762011d547..6af724e55b 100644 --- a/stats/rtcstats_objects.cc +++ b/stats/rtcstats_objects.cc @@ -936,10 +936,7 @@ WEBRTC_RTCSTATS_IMPL( "remote-outbound-rtp", &local_id, &remote_timestamp, - &reports_sent, - &round_trip_time, - &round_trip_time_measurements, - &total_round_trip_time) + &reports_sent) // clang-format on RTCRemoteOutboundRtpStreamStats::RTCRemoteOutboundRtpStreamStats( @@ -953,20 +950,14 @@ RTCRemoteOutboundRtpStreamStats::RTCRemoteOutboundRtpStreamStats( : RTCSentRtpStreamStats(std::move(id), timestamp_us), local_id("localId"), remote_timestamp("remoteTimestamp"), - reports_sent("reportsSent"), - round_trip_time("roundTripTime"), - round_trip_time_measurements("roundTripTimeMeasurements"), - total_round_trip_time("totalRoundTripTime") {} + reports_sent("reportsSent") {} RTCRemoteOutboundRtpStreamStats::RTCRemoteOutboundRtpStreamStats( const RTCRemoteOutboundRtpStreamStats& other) : RTCSentRtpStreamStats(other), local_id(other.local_id), remote_timestamp(other.remote_timestamp), - reports_sent(other.reports_sent), - round_trip_time(other.round_trip_time), - round_trip_time_measurements(other.round_trip_time_measurements), - total_round_trip_time(other.total_round_trip_time) {} + reports_sent(other.reports_sent) {} RTCRemoteOutboundRtpStreamStats::~RTCRemoteOutboundRtpStreamStats() {}