From 2562cf0105d914a3cebfdef4d818e76ff658387b Mon Sep 17 00:00:00 2001 From: Ivo Creusen Date: Fri, 3 Sep 2021 14:51:22 +0000 Subject: [PATCH] Reland "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 2c41cbae37cac548a1133589b9d2c2e8614fa6cb. Reason for revert: The breaking test in Chromium has been temporarily disabled in https://chromium-review.googlesource.com/c/chromium/src/+/3139794/2. Original change's description: > Revert "Wire up non-sender RTT for audio, and implement related standardized stats." > > 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} # Not skipping CQ checks because original CL landed > 1 day ago. Bug: webrtc:12951, webrtc:12714 Change-Id: I786b06933d85bdffc5e879bf52436bb3469b7f3a Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/231181 Reviewed-by: Henrik Boström Reviewed-by: Björn Terelius Reviewed-by: Harald Alvestrand Commit-Queue: Ivo Creusen Cr-Commit-Position: refs/heads/main@{#34930} --- 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, 552 insertions(+), 27 deletions(-) create mode 100644 audio/test/non_sender_rtt_test.cc diff --git a/api/stats/rtcstats_objects.h b/api/stats/rtcstats_objects.h index 8a6327ed4c..849ef80a5d 100644 --- a/api/stats/rtcstats_objects.h +++ b/api/stats/rtcstats_objects.h @@ -593,6 +593,9 @@ 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 4ecdcea9e7..ea6b9d2d55 100644 --- a/audio/BUILD.gn +++ b/audio/BUILD.gn @@ -142,6 +142,7 @@ 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 f3843812ee..6f2444901b 100644 --- a/audio/audio_receive_stream.cc +++ b/audio/audio_receive_stream.cc @@ -81,9 +81,10 @@ 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.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.enable_non_sender_rtt, + config.decoder_factory, config.codec_pair_id, + std::move(config.frame_decryptor), config.crypto_options, + std::move(config.frame_transformer)); } } // namespace @@ -242,6 +243,12 @@ 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, @@ -347,6 +354,9 @@ 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 61ebc2719f..444ec4586e 100644 --- a/audio/audio_receive_stream.h +++ b/audio/audio_receive_stream.h @@ -92,6 +92,7 @@ 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 db42efc373..58db5254fe 100644 --- a/audio/audio_send_stream_unittest.cc +++ b/audio/audio_send_stream_unittest.cc @@ -387,7 +387,8 @@ 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, " - "cng_payload_type: 42, red_payload_type: 43, payload_type: 103, " + "enable_non_sender_rtt: false, 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 b741a8c1ca..9c6f672c3f 100644 --- a/audio/channel_receive.cc +++ b/audio/channel_receive.cc @@ -97,6 +97,7 @@ 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, @@ -158,6 +159,7 @@ 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, @@ -524,6 +526,7 @@ 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, @@ -573,6 +576,7 @@ 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)); @@ -856,6 +860,15 @@ 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; } @@ -872,6 +885,11 @@ 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) { @@ -1110,6 +1128,7 @@ 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, @@ -1119,8 +1138,9 @@ 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, decoder_factory, codec_pair_id, - std::move(frame_decryptor), crypto_options, std::move(frame_transformer)); + jitter_buffer_enable_rtx_handling, enable_non_sender_rtt, 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 deec49feaf..d811e87719 100644 --- a/audio/channel_receive.h +++ b/audio/channel_receive.h @@ -74,6 +74,9 @@ 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 { @@ -138,6 +141,7 @@ 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, @@ -179,6 +183,7 @@ 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 ea2a2ac3f0..d445b51312 100644 --- a/audio/mock_voe_channel_proxy.h +++ b/audio/mock_voe_channel_proxy.h @@ -30,6 +30,7 @@ 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 new file mode 100644 index 0000000000..5c5b15eecf --- /dev/null +++ b/audio/test/non_sender_rtt_test.cc @@ -0,0 +1,58 @@ +/* + * 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 182cd49679..1c2d11b1af 100644 --- a/call/audio_receive_stream.h +++ b/call/audio_receive_stream.h @@ -96,6 +96,9 @@ 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 { @@ -115,6 +118,9 @@ class AudioReceiveStream : public MediaReceiveStream { NackConfig nack; } rtp; + // Receive-side RTT. + bool enable_non_sender_rtt = false; + Transport* rtcp_send_transport = nullptr; // NetEq settings. @@ -158,6 +164,8 @@ 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 916336b929..a36050a9f7 100644 --- a/call/audio_send_stream.cc +++ b/call/audio_send_stream.cc @@ -80,6 +80,8 @@ 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: " @@ -94,6 +96,7 @@ 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 e084d4219d..e38a47f871 100644 --- a/call/audio_send_stream.h +++ b/call/audio_send_stream.h @@ -140,6 +140,7 @@ 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 99a4dcd202..acfa09f039 100644 --- a/media/BUILD.gn +++ b/media/BUILD.gn @@ -66,6 +66,7 @@ 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 6467a442d6..2607a7a4aa 100644 --- a/media/base/media_channel.h +++ b/media/base/media_channel.h @@ -28,6 +28,7 @@ #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" @@ -531,6 +532,9 @@ 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 13401514bd..f61aab04f1 100644 --- a/media/engine/fake_webrtc_call.cc +++ b/media/engine/fake_webrtc_call.cc @@ -116,6 +116,10 @@ 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 aeef95477e..5f6b8643db 100644 --- a/media/engine/fake_webrtc_call.h +++ b/media/engine/fake_webrtc_call.h @@ -122,6 +122,7 @@ 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 0cf17ed984..75d1a5fe02 100644 --- a/media/engine/webrtc_voice_engine.cc +++ b/media/engine/webrtc_voice_engine.cc @@ -255,6 +255,7 @@ 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, @@ -278,6 +279,7 @@ 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; @@ -1245,6 +1247,11 @@ 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); @@ -1715,6 +1722,7 @@ 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; } @@ -1790,16 +1798,27 @@ 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) << "Recreate all the receive streams because the send " - "codec has changed."; + RTC_LOG(LS_INFO) << "Changing transport cc and NACK status on receive " + "streams."; 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; } @@ -1963,9 +1982,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_, sp.stream_ids(), recv_rtp_extensions_, this, - engine()->decoder_factory_, decoder_map_, codec_pair_id_, - engine()->audio_jitter_buffer_max_packets_, + 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_, engine()->audio_jitter_buffer_fast_accelerate_, engine()->audio_jitter_buffer_min_delay_ms_, engine()->audio_jitter_buffer_enable_rtx_handling_, @@ -2424,6 +2443,9 @@ 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 a7c2f396bb..a8eb61d318 100644 --- a/media/engine/webrtc_voice_engine.h +++ b/media/engine/webrtc_voice_engine.h @@ -277,6 +277,7 @@ 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 33c5a9bcf9..05e49fb861 100644 --- a/modules/rtp_rtcp/mocks/mock_rtp_rtcp.h +++ b/modules/rtp_rtcp/mocks/mock_rtp_rtcp.h @@ -152,6 +152,10 @@ 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 32f442a7f7..e64b6932ba 100644 --- a/modules/rtp_rtcp/source/rtcp_receiver.cc +++ b/modules/rtp_rtcp/source/rtcp_receiver.cc @@ -303,6 +303,15 @@ 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; @@ -435,6 +444,16 @@ 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; @@ -455,6 +474,7 @@ 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); @@ -463,7 +483,12 @@ bool RTCPReceiver::ParseCompoundPacket(rtc::ArrayView packet, HandleSdes(rtcp_block, packet_information); break; case rtcp::ExtendedReports::kPacketType: - HandleXr(rtcp_block, packet_information); + bool contains_dlrr; + uint32_t ssrc; + HandleXr(rtcp_block, packet_information, contains_dlrr, ssrc); + if (contains_dlrr) { + received_blocks[ssrc].dlrr = true; + } break; case rtcp::Bye::kPacketType: HandleBye(rtcp_block); @@ -515,6 +540,15 @@ 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_); @@ -832,18 +866,22 @@ void RTCPReceiver::HandleBye(const CommonHeader& rtcp_block) { } void RTCPReceiver::HandleXr(const CommonHeader& rtcp_block, - PacketInformation* packet_information) { + PacketInformation* packet_information, + bool& contains_dlrr, + uint32_t& ssrc) { 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(time_info); + HandleXrDlrrReportBlock(xr.sender_ssrc(), time_info); if (xr.target_bitrate()) { HandleXrTargetBitrate(xr.sender_ssrc(), *xr.target_bitrate(), @@ -872,7 +910,8 @@ void RTCPReceiver::HandleXrReceiveReferenceTime(uint32_t sender_ssrc, } } -void RTCPReceiver::HandleXrDlrrReportBlock(const rtcp::ReceiveTimeInfo& rti) { +void RTCPReceiver::HandleXrDlrrReportBlock(uint32_t sender_ssrc, + const rtcp::ReceiveTimeInfo& rti) { if (!registered_ssrcs_.contains(rti.ssrc)) // Not to us. return; @@ -884,14 +923,21 @@ void RTCPReceiver::HandleXrDlrrReportBlock(const rtcp::ReceiveTimeInfo& rti) { 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) + 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(); + } 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 206b63ae8f..f45b783701 100644 --- a/modules/rtp_rtcp/source/rtcp_receiver.h +++ b/modules/rtp_rtcp/source/rtcp_receiver.h @@ -16,8 +16,10 @@ #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" @@ -58,6 +60,35 @@ 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); @@ -108,6 +139,9 @@ 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); @@ -277,14 +311,16 @@ class RTCPReceiver final { RTC_EXCLUSIVE_LOCKS_REQUIRED(rtcp_receiver_lock_); void HandleXr(const rtcp::CommonHeader& rtcp_block, - PacketInformation* packet_information) + PacketInformation* packet_information, + bool& contains_dlrr, + uint32_t& ssrc) 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(const rtcp::ReceiveTimeInfo& rti) + void HandleXrDlrrReportBlock(uint32_t ssrc, const rtcp::ReceiveTimeInfo& rti) RTC_EXCLUSIVE_LOCKS_REQUIRED(rtcp_receiver_lock_); void HandleXrTargetBitrate(uint32_t ssrc, @@ -382,6 +418,9 @@ 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 585d6980e6..fa7d569012 100644 --- a/modules/rtp_rtcp/source/rtcp_receiver_unittest.cc +++ b/modules/rtp_rtcp/source/rtcp_receiver_unittest.cc @@ -802,6 +802,11 @@ 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) { @@ -826,6 +831,11 @@ 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) { @@ -851,6 +861,11 @@ 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) { @@ -901,6 +916,11 @@ 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) { @@ -912,6 +932,11 @@ 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) { @@ -938,6 +963,12 @@ 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 @@ -967,6 +998,12 @@ 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 @@ -996,6 +1033,11 @@ 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) { @@ -1022,6 +1064,205 @@ 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 a9bd671a34..3dda8c6710 100644 --- a/modules/rtp_rtcp/source/rtp_rtcp_impl.cc +++ b/modules/rtp_rtcp/source/rtp_rtcp_impl.cc @@ -603,6 +603,12 @@ 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 2ffe013483..655691c9e1 100644 --- a/modules/rtp_rtcp/source/rtp_rtcp_impl.h +++ b/modules/rtp_rtcp/source/rtp_rtcp_impl.h @@ -200,6 +200,9 @@ 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 88de8db387..73ae138085 100644 --- a/modules/rtp_rtcp/source/rtp_rtcp_impl2.cc +++ b/modules/rtp_rtcp/source/rtp_rtcp_impl2.cc @@ -552,6 +552,17 @@ 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 a5038956ed..57a3fd1391 100644 --- a/modules/rtp_rtcp/source/rtp_rtcp_impl2.h +++ b/modules/rtp_rtcp/source/rtp_rtcp_impl2.h @@ -213,6 +213,7 @@ 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 70c9a1cf40..57b564acf4 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 sender report stats are not available if no RTCP SR was sent. +// Checks that the remote sender 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 sender report stats are available if an RTCP SR was sent. +// Checks that the remote sender 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 sender report stats are not available if an RTCP SR with an +// Checks that the remote sender 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 sender report stats count equals the number of sent RTCP SRs. +// Checks that the remote sender 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 sender report stats include a valid arrival time if an RTCP +// Checks that the remote sender 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 df5593cc54..13ebb0ab63 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 NPT timestamp for the last received RTCP SR. + // Arrival NTP 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,6 +170,16 @@ 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 @@ -403,6 +413,8 @@ 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 c2b453ef00..1f8b28df62 100644 --- a/pc/rtc_stats_collector.cc +++ b/pc/rtc_stats_collector.cc @@ -436,6 +436,14 @@ 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 6af724e55b..762011d547 100644 --- a/stats/rtcstats_objects.cc +++ b/stats/rtcstats_objects.cc @@ -936,7 +936,10 @@ WEBRTC_RTCSTATS_IMPL( "remote-outbound-rtp", &local_id, &remote_timestamp, - &reports_sent) + &reports_sent, + &round_trip_time, + &round_trip_time_measurements, + &total_round_trip_time) // clang-format on RTCRemoteOutboundRtpStreamStats::RTCRemoteOutboundRtpStreamStats( @@ -950,14 +953,20 @@ RTCRemoteOutboundRtpStreamStats::RTCRemoteOutboundRtpStreamStats( : RTCSentRtpStreamStats(std::move(id), timestamp_us), local_id("localId"), remote_timestamp("remoteTimestamp"), - reports_sent("reportsSent") {} + reports_sent("reportsSent"), + round_trip_time("roundTripTime"), + round_trip_time_measurements("roundTripTimeMeasurements"), + total_round_trip_time("totalRoundTripTime") {} RTCRemoteOutboundRtpStreamStats::RTCRemoteOutboundRtpStreamStats( const RTCRemoteOutboundRtpStreamStats& other) : RTCSentRtpStreamStats(other), local_id(other.local_id), remote_timestamp(other.remote_timestamp), - reports_sent(other.reports_sent) {} + 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) {} RTCRemoteOutboundRtpStreamStats::~RTCRemoteOutboundRtpStreamStats() {}