Enable non-sender RTT from negotiated rcvr-rtt; report video RTT Drive RRTR/DLRR enablement from the negotiated receive_non_sender_rtt flag (set by the SDP layer from either rtcp-xr:rcvr-rtt or the legacy rtcp-fb rrtr) instead of HasRrtr, for both the audio and video receive channels. Enablement uses SetReceiveNonSenderRttEnabled on each receive channel: receive streams can be created before the receive parameters are finalized (e.g. a recvonly endpoint creates them while processing the offer), so the value cannot reliably be set at stream construction and must be applied via the setter, as the voice channel already does. Also surfaces the computed video non-sender RTT into RTCRemoteOutboundRtpStreamStats (GetNonSenderRttStats -> VideoReceiveStream2::Stats -> VideoReceiverInfo), resolving the long-standing TODO(bugs.webrtc.org/12529). Bug: webrtc:516205747 Change-Id: I01da1c79389e4818ca8023c2f248e32b1b84aa2c Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/479241 Reviewed-by: Danil Chapovalov <danilchap@webrtc.org> Reviewed-by: Harald Alvestrand <hta@webrtc.org> Commit-Queue: Danil Chapovalov <danilchap@webrtc.org> Cr-Commit-Position: refs/heads/main@{#48157}
diff --git a/call/video_receive_stream.h b/call/video_receive_stream.h index a227891..cb2879d 100644 --- a/call/video_receive_stream.h +++ b/call/video_receive_stream.h
@@ -185,6 +185,11 @@ uint32_t sender_reports_packets_sent = 0; uint64_t sender_reports_bytes_sent = 0; uint64_t sender_reports_reports_count = 0; + // Non-sender RTT (RRTR/DLRR), surfaced into + // RTCRemoteOutboundRtpStreamStats for recvonly endpoints. + std::optional<TimeDelta> round_trip_time; + TimeDelta total_round_trip_time = TimeDelta::Zero(); + int round_trip_time_measurements = 0; }; struct Config {
diff --git a/media/base/fake_media_engine.h b/media/base/fake_media_engine.h index cc3c6d7..2f94796 100644 --- a/media/base/fake_media_engine.h +++ b/media/base/fake_media_engine.h
@@ -718,6 +718,7 @@ bool HasSink(uint32_t ssrc) const; void SetReceive(bool /* receive */) override {} + void SetReceiveNonSenderRttEnabled(bool /* enabled */) override {} bool HasSource(uint32_t ssrc) const; bool AddRecvStream(const StreamParams& sp) override;
diff --git a/media/base/media_channel.h b/media/base/media_channel.h index 4bc9b30..214e284 100644 --- a/media/base/media_channel.h +++ b/media/base/media_channel.h
@@ -1065,6 +1065,7 @@ virtual absl::AnyInvocable<std::optional<VideoMediaReceiveInfo>()> GetStatsTask() = 0; virtual bool AddDefaultRecvStreamForTesting(const StreamParams& sp) = 0; + virtual void SetReceiveNonSenderRttEnabled(bool enabled) = 0; }; } // namespace webrtc
diff --git a/media/engine/webrtc_video_engine.cc b/media/engine/webrtc_video_engine.cc index a66f4fa..256a5aa 100644 --- a/media/engine/webrtc_video_engine.cc +++ b/media/engine/webrtc_video_engine.cc
@@ -3004,6 +3004,17 @@ receiving_ = receive; } +void WebRtcVideoReceiveChannel::SetReceiveNonSenderRttEnabled(bool enabled) { + RTC_DCHECK_RUN_ON(&thread_checker_); + if (enable_non_sender_rtt_ == enabled) { + return; + } + enable_non_sender_rtt_ = enabled; + for (auto& kv : receive_streams_) { + kv.second->SetNonSenderRttMeasurement(enabled); + } +} + bool WebRtcVideoReceiveChannel::ValidateReceiveSsrcAvailability( const StreamParams& sp) const { for (uint32_t ssrc : sp.ssrcs) { @@ -3078,6 +3089,7 @@ if (unsignaled_frame_transformer_ && !config.frame_transformer) config.frame_transformer = unsignaled_frame_transformer_; + config.rtp.rtcp_xr.receiver_reference_time_report = enable_non_sender_rtt_; auto receive_stream = new WebRtcVideoReceiveStream( env_, call_, sp, std::move(config), default_stream, recv_codecs_, flexfec_config); @@ -3507,8 +3519,6 @@ config_.rtp.nack.rtp_history_ms = *codec.rtx_time; } - config_.rtp.rtcp_xr.receiver_reference_time_report = HasRrtr(codec.codec); - if (codec.ulpfec.red_rtx_payload_type != -1) { config_.rtp .rtx_associated_payload_types[codec.ulpfec.red_rtx_payload_type] = @@ -3603,12 +3613,6 @@ stream_->SetNackHistory(TimeDelta::Millis(new_history_ms)); } - const bool has_rtr = HasRrtr(codec.codec); - if (has_rtr != config_.rtp.rtcp_xr.receiver_reference_time_report) { - config_.rtp.rtcp_xr.receiver_reference_time_report = has_rtr; - stream_->SetRtcpXr(config_.rtp.rtcp_xr); - } - if (codec.ulpfec.red_rtx_payload_type != -1) { rtx_associated_payload_types[codec.ulpfec.red_rtx_payload_type] = codec.ulpfec.red_payload_type; @@ -3795,6 +3799,17 @@ } } +void WebRtcVideoReceiveChannel::WebRtcVideoReceiveStream:: + SetNonSenderRttMeasurement(bool enabled) { + RTC_DCHECK_RUN_ON(&thread_checker_); + if (config_.rtp.rtcp_xr.receiver_reference_time_report != enabled) { + config_.rtp.rtcp_xr.receiver_reference_time_report = enabled; + if (stream_) { + stream_->SetRtcpXr(config_.rtp.rtcp_xr); + } + } +} + bool WebRtcVideoReceiveChannel::WebRtcVideoReceiveStream:: SetBaseMinimumPlayoutDelayMs(int delay_ms) { return stream_ ? stream_->SetBaseMinimumPlayoutDelayMs(delay_ms) : false; @@ -3956,8 +3971,11 @@ info.sender_reports_packets_sent = stats.sender_reports_packets_sent; info.sender_reports_bytes_sent = stats.sender_reports_bytes_sent; info.sender_reports_reports_count = stats.sender_reports_reports_count; - // TODO(bugs.webrtc.org/12529): RTT-related fields are missing and can only be - // present if DLRR is enabled. + // Non-sender RTT (RRTR/DLRR), surfaced for recvonly endpoints; mirrors the + // audio path. + info.round_trip_time = stats.round_trip_time; + info.round_trip_time_measurements = stats.round_trip_time_measurements; + info.total_round_trip_time = stats.total_round_trip_time; if (log_stats) RTC_LOG(LS_INFO) << stats.ToString(env_.clock().TimeInMilliseconds(),
diff --git a/media/engine/webrtc_video_engine.h b/media/engine/webrtc_video_engine.h index 5c9af49..00279e4 100644 --- a/media/engine/webrtc_video_engine.h +++ b/media/engine/webrtc_video_engine.h
@@ -509,6 +509,7 @@ RtpParameters GetRtpReceiverParameters(uint32_t ssrc) const override; RtpParameters GetDefaultRtpReceiveParameters() const override; void SetReceive(bool receive) override; + void SetReceiveNonSenderRttEnabled(bool enabled) override; bool AddRecvStream(const StreamParams& sp) override; bool AddDefaultRecvStreamForTesting(const StreamParams& sp) override { // Invokes private AddRecvStream variant function @@ -609,6 +610,7 @@ RtpParameters GetRtpParameters() const; void SetReceiverParameters(const ChangedReceiverParameters& recv_params); + void SetNonSenderRttMeasurement(bool enabled); void OnFrame(const VideoFrame& frame) override; bool IsDefaultStream() const; @@ -680,6 +682,7 @@ std::map<uint32_t, WebRtcVideoReceiveStream*> receive_streams_ RTC_GUARDED_BY(thread_checker_); + bool enable_non_sender_rtt_ RTC_GUARDED_BY(thread_checker_) = false; void FillReceiverStats(VideoMediaReceiveInfo* info, bool log_stats) RTC_EXCLUSIVE_LOCKS_REQUIRED(thread_checker_); void FillReceiveCodecStats(VideoMediaReceiveInfo* video_media_info)
diff --git a/media/engine/webrtc_video_engine_unittest.cc b/media/engine/webrtc_video_engine_unittest.cc index 897e79b..a2303c8 100644 --- a/media/engine/webrtc_video_engine_unittest.cc +++ b/media/engine/webrtc_video_engine_unittest.cc
@@ -3156,6 +3156,26 @@ uint32_t last_ssrc_; }; +// SetReceiveNonSenderRttEnabled may toggle the value after a receive stream +// has started: false->true enables it, true->false disables it. +TEST_F(WebRtcVideoChannelTest, ToggleReceiveNonSenderRttAfterStreamStart) { + FakeVideoReceiveStream* recv_stream = + AddRecvStream(StreamParams::CreateLegacy(kSsrc)); + // Not negotiated yet -> disabled. + EXPECT_FALSE( + recv_stream->GetConfig().rtp.rtcp_xr.receiver_reference_time_report); + + // false -> true: enabled on the already-running stream. + receive_channel_->SetReceiveNonSenderRttEnabled(true); + EXPECT_TRUE( + recv_stream->GetConfig().rtp.rtcp_xr.receiver_reference_time_report); + + // true -> false: disabled immediately. + receive_channel_->SetReceiveNonSenderRttEnabled(false); + EXPECT_FALSE( + recv_stream->GetConfig().rtp.rtcp_xr.receiver_reference_time_report); +} + TEST_F(WebRtcVideoChannelTest, SetsSyncGroupFromSyncLabel) { const uint32_t kVideoSsrc = 123; const std::string kSyncLabel = "AvSyncLabel";
diff --git a/pc/channel.cc b/pc/channel.cc index e0b06b8..f1579ef 100644 --- a/pc/channel.cc +++ b/pc/channel.cc
@@ -843,12 +843,22 @@ : RtcpMode::kCompound); voice_media_receive_channel()->SetReceiveNackEnabled( voice_media_send_channel()->SenderNackEnabled()); + // Enable non-sender RTT (RRTR/DLRR) when negotiated. Both the standard + // a=rtcp-xr:rcvr-rtt and the legacy rtcp-fb rrtr are folded into + // receive_non_sender_rtt by the SDP layer. voice_media_receive_channel()->SetReceiveNonSenderRttEnabled( - voice_media_send_channel()->SenderNonSenderRttEnabled()); + content->receive_non_sender_rtt()); } RTC_DCHECK_BLOCK_COUNT_NO_MORE_THAN(0); + if (media_type_ == MediaType::VIDEO) { + // Enable non-sender RTT (RRTR/DLRR) when negotiated (both wire forms are + // folded into receive_non_sender_rtt by the SDP layer). + video_media_receive_channel()->SetReceiveNonSenderRttEnabled( + content->receive_non_sender_rtt()); + } + error = UpdateRemoteStreams_w(content, type); RTC_DCHECK_BLOCK_COUNT_NO_MORE_THAN(1);
diff --git a/pc/rtc_stats_integrationtest.cc b/pc/rtc_stats_integrationtest.cc index a0e164a..4c82891 100644 --- a/pc/rtc_stats_integrationtest.cc +++ b/pc/rtc_stats_integrationtest.cc
@@ -1089,6 +1089,90 @@ RTCStatsReportVerifier(report.get()).VerifyReport({}); } +// A recvonly audio endpoint measures non-sender RTT (RRTR/DLRR) once rcvr-rtt +// is negotiated, surfacing round_trip_time on remote-outbound-rtp. The SDP +// negotiation itself is covered by the negotiation CL. +TEST_F(RTCStatsIntegrationTest, RcvrRttNegotiatedEnablesAudioRtt) { + PeerConnectionInterface::RTCConfiguration config; + config.sdp_semantics = SdpSemantics::kUnifiedPlan; + PeerConnectionInterface::IceServer ice_server; + ice_server.uri = "stun:1.1.1.1:3478"; + config.servers.push_back(ice_server); + EXPECT_TRUE(caller_->CreatePc(config, CreateBuiltinAudioEncoderFactory(), + CreateBuiltinAudioDecoderFactory())); + EXPECT_TRUE(callee_->CreatePc(config, CreateBuiltinAudioEncoderFactory(), + CreateBuiltinAudioDecoderFactory())); + PeerConnectionTestWrapper::Connect(caller_.get(), callee_.get()); + + // Caller sends audio; callee adds nothing, so it is negotiated recvonly. + caller_->GetAndAddUserMedia(true, AudioOptions(), false); + caller_->CreateOffer(PeerConnectionInterface::RTCOfferAnswerOptions()); + EXPECT_TRUE(caller_->WaitForConnection()); + EXPECT_TRUE(callee_->WaitForConnection()); + + // The recvonly callee should measure non-sender RTT. + // TODO(bugs.webrtc.org/516205747): runs against the virtual socket server + // with a real clock (a valid RTT needs an RRTR exchange plus three SR + // blocks), so this takes a few seconds; switch to a simulated clock when + // the fixture supports it. + scoped_refptr<const RTCStatsReport> report; + constexpr TimeDelta kMaxWait = TimeDelta::Seconds(10); + EXPECT_TRUE(WaitUntil( + [&] { + report = GetStatsFromCallee(); + for (const auto* stats : + report->GetStatsOfType<RTCRemoteOutboundRtpStreamStats>()) { + if (stats->round_trip_time_measurements.value_or(0) > 0u) { + return true; + } + } + return false; + }, + {.timeout = kMaxWait})); +} + +// With rcvr-rtt negotiated, a recvonly endpoint must report non-sender RTT on +// both the audio and video remote-outbound-rtp stats (video resolves the +// bugs.webrtc.org/12529 stats gap). +TEST_F(RTCStatsIntegrationTest, RcvrRttNegotiatedEnablesVideoRtt) { + PeerConnectionInterface::RTCConfiguration config; + config.sdp_semantics = SdpSemantics::kUnifiedPlan; + PeerConnectionInterface::IceServer ice_server; + ice_server.uri = "stun:1.1.1.1:3478"; + config.servers.push_back(ice_server); + EXPECT_TRUE(caller_->CreatePc(config, CreateBuiltinAudioEncoderFactory(), + CreateBuiltinAudioDecoderFactory())); + EXPECT_TRUE(callee_->CreatePc(config, CreateBuiltinAudioEncoderFactory(), + CreateBuiltinAudioDecoderFactory())); + PeerConnectionTestWrapper::Connect(caller_.get(), callee_.get()); + + // Caller sends audio and video; callee adds nothing, so it is recvonly. + caller_->GetAndAddUserMedia(true, AudioOptions(), true); + caller_->CreateOffer(PeerConnectionInterface::RTCOfferAnswerOptions()); + EXPECT_TRUE(caller_->WaitForConnection()); + EXPECT_TRUE(callee_->WaitForConnection()); + + // TODO(bugs.webrtc.org/516205747): Switch to a simulated clock when the + // fixture supports it. See the audio test above for more details. + scoped_refptr<const RTCStatsReport> report; + constexpr TimeDelta kMaxWait = TimeDelta::Seconds(10); + // Both the audio and video remote-outbound-rtp must report a non-sender + // RTT measurement, so at least two streams carry one. + EXPECT_TRUE(WaitUntil( + [&] { + report = GetStatsFromCallee(); + int with_rtt = 0; + for (const auto* stats : + report->GetStatsOfType<RTCRemoteOutboundRtpStreamStats>()) { + if (stats->round_trip_time_measurements.value_or(0) > 0u) { + ++with_rtt; + } + } + return with_rtt >= 2; + }, + {.timeout = kMaxWait})); +} + // These tests exercise the integration of the stats selection algorithm inside // of PeerConnection. See rtcstatstraveral_unittest.cc for more detailed stats // traversal tests on particular stats graphs.
diff --git a/video/rtp_video_stream_receiver2.cc b/video/rtp_video_stream_receiver2.cc index f228fc4..20bbc32 100644 --- a/video/rtp_video_stream_receiver2.cc +++ b/video/rtp_video_stream_receiver2.cc
@@ -1142,6 +1142,12 @@ return rtp_rtcp_->GetSenderReportStats(); } +std::optional<RtpRtcpInterface::NonSenderRttStats> +RtpVideoStreamReceiver2::GetNonSenderRttStats() const { + RTC_DCHECK_RUN_ON(&packet_sequence_checker_); + return rtp_rtcp_->GetNonSenderRttStats(); +} + std::optional<VideoCodecType> RtpVideoStreamReceiver2::GetCodecFromPayloadType( uint8_t payload_type) const { RTC_DCHECK_RUN_ON(&packet_sequence_checker_);
diff --git a/video/rtp_video_stream_receiver2.h b/video/rtp_video_stream_receiver2.h index 06e53a6..d43c031 100644 --- a/video/rtp_video_stream_receiver2.h +++ b/video/rtp_video_stream_receiver2.h
@@ -227,6 +227,8 @@ std::optional<RtpRtcpInterface::SenderReportStats> GetSenderReportStats() const; + std::optional<RtpRtcpInterface::NonSenderRttStats> GetNonSenderRttStats() + const; private: // Implements RtpVideoFrameReceiver.
diff --git a/video/video_receive_stream2.cc b/video/video_receive_stream2.cc index 7c1b81b..2dc761a 100644 --- a/video/video_receive_stream2.cc +++ b/video/video_receive_stream2.cc
@@ -612,6 +612,17 @@ stats.sender_reports_bytes_sent = rtcp_sr_stats->bytes_sent; stats.sender_reports_reports_count = rtcp_sr_stats->reports_count; } + + // Non-sender RTT (RRTR/DLRR) for recvonly endpoints, reported via + // RTCRemoteOutboundRtpStreamStats round-trip-time fields. + std::optional<RtpRtcpInterface::NonSenderRttStats> non_sender_rtt_stats = + rtp_video_stream_receiver_.GetNonSenderRttStats(); + if (non_sender_rtt_stats) { + 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; }