From 558c2dc5397cbb85db138ef17e36c816c4dad50e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Olov=20Br=C3=A4ndstr=C3=B6m?= Date: Tue, 15 Oct 2024 15:10:52 +0200 Subject: [PATCH] Change timestamps type from int64 to Timestamp in MediaReceiverInfo. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We should use the Timestamp type, rather then int64, to store timestamps. In https://webrtc-review.googlesource.com/c/src/+/365001/ an additional int64 timestamp was added (last_sender_report_timestamp_ms). This CL fixes the new timestamp, as well as other similar timestamps in MediaReceiverInfo (last_sender_report_utc_timestamp_ms and last_sender_report_remote_utc_timestamp_ms). Bug: webrtc:372393493 Change-Id: I0e473730e85a69ec595b421e2c3db920364008eb Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/365641 Reviewed-by: Harald Alvestrand Commit-Queue: Olov Brändström Reviewed-by: Danil Chapovalov Reviewed-by: Henrik Boström Cr-Commit-Position: refs/heads/main@{#43248} --- audio/audio_receive_stream.cc | 11 +++++------ audio/channel_receive.cc | 15 +++++++-------- audio/channel_receive.h | 8 +++----- call/audio_receive_stream.h | 6 +++--- call/video_receive_stream.h | 6 +++--- media/base/media_channel.h | 8 +++----- media/engine/webrtc_video_engine.cc | 10 +++++----- media/engine/webrtc_voice_engine.cc | 11 +++++------ pc/rtc_stats_collector.cc | 18 +++++++++--------- pc/rtc_stats_collector_unittest.cc | 18 +++++++++--------- video/video_receive_stream2.cc | 12 ++++++------ 11 files changed, 58 insertions(+), 65 deletions(-) diff --git a/audio/audio_receive_stream.cc b/audio/audio_receive_stream.cc index 6edc7d03f1..48e1386413 100644 --- a/audio/audio_receive_stream.cc +++ b/audio/audio_receive_stream.cc @@ -332,12 +332,11 @@ webrtc::AudioReceiveStreamInterface::Stats AudioReceiveStreamImpl::GetStats( stats.decoding_plc_cng = ds.decoded_plc_cng; stats.decoding_muted_output = ds.decoded_muted_output; - stats.last_sender_report_timestamp_ms = - call_stats.last_sender_report_timestamp_ms; - stats.last_sender_report_utc_timestamp_ms = - call_stats.last_sender_report_utc_timestamp_ms; - stats.last_sender_report_remote_utc_timestamp_ms = - call_stats.last_sender_report_remote_utc_timestamp_ms; + stats.last_sender_report_timestamp = call_stats.last_sender_report_timestamp; + stats.last_sender_report_utc_timestamp = + call_stats.last_sender_report_utc_timestamp; + stats.last_sender_report_remote_utc_timestamp = + call_stats.last_sender_report_remote_utc_timestamp; 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; diff --git a/audio/channel_receive.cc b/audio/channel_receive.cc index 865fe00442..9fc2882258 100644 --- a/audio/channel_receive.cc +++ b/audio/channel_receive.cc @@ -872,14 +872,13 @@ CallReceiveStatistics ChannelReceive::GetRTCPStatistics() const { std::optional rtcp_sr_stats = rtp_rtcp_->GetSenderReportStats(); if (rtcp_sr_stats.has_value()) { - stats.last_sender_report_timestamp_ms = - rtcp_sr_stats->last_arrival_timestamp.ms(); - stats.last_sender_report_utc_timestamp_ms = - rtcp_sr_stats->last_arrival_ntp_timestamp.ToMs() - - rtc::kNtpJan1970Millisecs; - stats.last_sender_report_remote_utc_timestamp_ms = - rtcp_sr_stats->last_remote_ntp_timestamp.ToMs() - - rtc::kNtpJan1970Millisecs; + stats.last_sender_report_timestamp = rtcp_sr_stats->last_arrival_timestamp; + stats.last_sender_report_utc_timestamp = + Timestamp::Millis(rtcp_sr_stats->last_arrival_ntp_timestamp.ToMs() - + rtc::kNtpJan1970Millisecs); + stats.last_sender_report_remote_utc_timestamp = + Timestamp::Millis(rtcp_sr_stats->last_remote_ntp_timestamp.ToMs() - + rtc::kNtpJan1970Millisecs); stats.sender_reports_packets_sent = rtcp_sr_stats->packets_sent; stats.sender_reports_bytes_sent = rtcp_sr_stats->bytes_sent; stats.sender_reports_reports_count = rtcp_sr_stats->reports_count; diff --git a/audio/channel_receive.h b/audio/channel_receive.h index ab6ce531d6..fbc7f1eea7 100644 --- a/audio/channel_receive.h +++ b/audio/channel_receive.h @@ -67,13 +67,11 @@ struct CallReceiveStatistics { // Note that the timestamps below correspond to the time elapsed since the // Unix epoch. // https://w3c.github.io/webrtc-stats/#remoteoutboundrtpstats-dict* - // TODO: bugs.webrtc.org/372393493 - timestamps should use the type Timestamp, - // not int64_t. - std::optional last_sender_report_timestamp_ms; + std::optional last_sender_report_timestamp; // TODO: bugs.webrtc.org/370535296 - Remove the utc timestamp when linked // issue is fixed. - std::optional last_sender_report_utc_timestamp_ms; - std::optional last_sender_report_remote_utc_timestamp_ms; + std::optional last_sender_report_utc_timestamp; + std::optional last_sender_report_remote_utc_timestamp; uint64_t sender_reports_packets_sent = 0; uint64_t sender_reports_bytes_sent = 0; uint64_t sender_reports_reports_count = 0; diff --git a/call/audio_receive_stream.h b/call/audio_receive_stream.h index 8056e9c0d9..9e64521796 100644 --- a/call/audio_receive_stream.h +++ b/call/audio_receive_stream.h @@ -100,11 +100,11 @@ class AudioReceiveStreamInterface : public MediaReceiveStreamInterface { std::optional estimated_playout_ntp_timestamp_ms; // Remote outbound stats derived by the received RTCP sender reports. // https://w3c.github.io/webrtc-stats/#remoteoutboundrtpstats-dict* - std::optional last_sender_report_timestamp_ms; + std::optional last_sender_report_timestamp; // TODO: bugs.webrtc.org/370535296 - Remove the utc timestamp when linked // issue is fixed. - std::optional last_sender_report_utc_timestamp_ms; - std::optional last_sender_report_remote_utc_timestamp_ms; + std::optional last_sender_report_utc_timestamp; + std::optional last_sender_report_remote_utc_timestamp; uint64_t sender_reports_packets_sent = 0; uint64_t sender_reports_bytes_sent = 0; uint64_t sender_reports_reports_count = 0; diff --git a/call/video_receive_stream.h b/call/video_receive_stream.h index 9de6f43156..f13a006e1e 100644 --- a/call/video_receive_stream.h +++ b/call/video_receive_stream.h @@ -175,11 +175,11 @@ class VideoReceiveStreamInterface : public MediaReceiveStreamInterface { // Remote outbound stats derived by the received RTCP sender reports. // https://w3c.github.io/webrtc-stats/#remoteoutboundrtpstats-dict* - std::optional last_sender_report_timestamp_ms; + std::optional last_sender_report_timestamp; // TODO: bugs.webrtc.org/370535296 - Remove the utc timestamp when linked // issue is fixed. - std::optional last_sender_report_utc_timestamp_ms; - std::optional last_sender_report_remote_utc_timestamp_ms; + std::optional last_sender_report_utc_timestamp; + std::optional last_sender_report_remote_utc_timestamp; uint32_t sender_reports_packets_sent = 0; uint64_t sender_reports_bytes_sent = 0; uint64_t sender_reports_reports_count = 0; diff --git a/media/base/media_channel.h b/media/base/media_channel.h index 73d7015ab4..efd3693052 100644 --- a/media/base/media_channel.h +++ b/media/base/media_channel.h @@ -482,13 +482,11 @@ struct MediaReceiverInfo { // Remote outbound stats derived by the received RTCP sender reports. // https://w3c.github.io/webrtc-stats/#remoteoutboundrtpstats-dict* - // TODO: bugs.webrtc.org/372393493 - timestamps should use the type Timestamp, - // not int64_t. - std::optional last_sender_report_timestamp_ms; + std::optional last_sender_report_timestamp; // TODO: bugs.webrtc.org/370535296 - Remove the utc timestamp when linked // issue is fixed. - std::optional last_sender_report_utc_timestamp_ms; - std::optional last_sender_report_remote_utc_timestamp_ms; + std::optional last_sender_report_utc_timestamp; + std::optional last_sender_report_remote_utc_timestamp; uint64_t sender_reports_packets_sent = 0; uint64_t sender_reports_bytes_sent = 0; uint64_t sender_reports_reports_count = 0; diff --git a/media/engine/webrtc_video_engine.cc b/media/engine/webrtc_video_engine.cc index a57af55409..bbcaf1b52d 100644 --- a/media/engine/webrtc_video_engine.cc +++ b/media/engine/webrtc_video_engine.cc @@ -3845,11 +3845,11 @@ WebRtcVideoReceiveChannel::WebRtcVideoReceiveStream::GetVideoReceiverInfo( } // remote-outbound-rtp stats. - info.last_sender_report_timestamp_ms = stats.last_sender_report_timestamp_ms; - info.last_sender_report_utc_timestamp_ms = - stats.last_sender_report_utc_timestamp_ms; - info.last_sender_report_remote_utc_timestamp_ms = - stats.last_sender_report_remote_utc_timestamp_ms; + info.last_sender_report_timestamp = stats.last_sender_report_timestamp; + info.last_sender_report_utc_timestamp = + stats.last_sender_report_utc_timestamp; + info.last_sender_report_remote_utc_timestamp = + stats.last_sender_report_remote_utc_timestamp; 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; diff --git a/media/engine/webrtc_voice_engine.cc b/media/engine/webrtc_voice_engine.cc index 3dda76a873..0f83d92bd3 100644 --- a/media/engine/webrtc_voice_engine.cc +++ b/media/engine/webrtc_voice_engine.cc @@ -2697,12 +2697,11 @@ bool WebRtcVoiceReceiveChannel::GetStats(VoiceMediaReceiveInfo* info, stats.relative_packet_arrival_delay_seconds; rinfo.interruption_count = stats.interruption_count; rinfo.total_interruption_duration_ms = stats.total_interruption_duration_ms; - rinfo.last_sender_report_timestamp_ms = - stats.last_sender_report_timestamp_ms; - rinfo.last_sender_report_utc_timestamp_ms = - stats.last_sender_report_utc_timestamp_ms; - rinfo.last_sender_report_remote_utc_timestamp_ms = - stats.last_sender_report_remote_utc_timestamp_ms; + rinfo.last_sender_report_timestamp = stats.last_sender_report_timestamp; + rinfo.last_sender_report_utc_timestamp = + stats.last_sender_report_utc_timestamp; + rinfo.last_sender_report_remote_utc_timestamp = + stats.last_sender_report_remote_utc_timestamp; 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; diff --git a/pc/rtc_stats_collector.cc b/pc/rtc_stats_collector.cc index 60265f4093..60741b64ce 100644 --- a/pc/rtc_stats_collector.cc +++ b/pc/rtc_stats_collector.cc @@ -523,11 +523,11 @@ CreateRemoteOutboundMediaStreamStats( const RTCInboundRtpStreamStats& inbound_audio_stats, const std::string& transport_id, const bool stats_timestamp_with_environment_clock) { - std::optional last_sender_report_timestamp_ms = + std::optional last_sender_report_timestamp = stats_timestamp_with_environment_clock - ? media_receiver_info.last_sender_report_timestamp_ms - : media_receiver_info.last_sender_report_utc_timestamp_ms; - if (!last_sender_report_timestamp_ms.has_value()) { + ? media_receiver_info.last_sender_report_timestamp + : media_receiver_info.last_sender_report_utc_timestamp; + if (!last_sender_report_timestamp.has_value()) { // Cannot create `RTCRemoteOutboundRtpStreamStats` when the RTCP SR arrival // timestamp is not available - i.e., until the first sender report is // received. @@ -539,7 +539,7 @@ CreateRemoteOutboundMediaStreamStats( auto stats = std::make_unique( /*id=*/RTCRemoteOutboundRTPStreamStatsIDFromSSRC( media_type, media_receiver_info.ssrc()), - Timestamp::Millis(*last_sender_report_timestamp_ms)); + *last_sender_report_timestamp); // Populate. // - RTCRtpStreamStats. @@ -556,10 +556,10 @@ CreateRemoteOutboundMediaStreamStats( stats->local_id = inbound_audio_stats.id(); // last_sender_report_remote_utc_timestamp_ms is set together with // last_sender_report_utc_timestamp_ms. - RTC_DCHECK(media_receiver_info.last_sender_report_remote_utc_timestamp_ms - .has_value()); - stats->remote_timestamp = static_cast( - *media_receiver_info.last_sender_report_remote_utc_timestamp_ms); + RTC_DCHECK( + media_receiver_info.last_sender_report_remote_utc_timestamp.has_value()); + stats->remote_timestamp = + media_receiver_info.last_sender_report_remote_utc_timestamp->ms(); stats->reports_sent = media_receiver_info.sender_reports_reports_count; if (media_receiver_info.round_trip_time.has_value()) { stats->round_trip_time = diff --git a/pc/rtc_stats_collector_unittest.cc b/pc/rtc_stats_collector_unittest.cc index fe999ef44a..e7fdb1abd9 100644 --- a/pc/rtc_stats_collector_unittest.cc +++ b/pc/rtc_stats_collector_unittest.cc @@ -145,8 +145,9 @@ const int64_t kGetStatsReportTimeoutMs = 1000; // Fake data used by `SetupExampleStatsVoiceGraph()` to fill in remote outbound // stats. -constexpr int64_t kRemoteOutboundStatsTimestampMs = 123; -constexpr int64_t kRemoteOutboundStatsRemoteTimestampMs = 456; +constexpr Timestamp kRemoteOutboundStatsTimestamp = Timestamp::Millis(123); +constexpr Timestamp kRemoteOutboundStatsRemoteTimestamp = + Timestamp::Millis(456); constexpr uint32_t kRemoteOutboundStatsPacketsSent = 7u; constexpr uint64_t kRemoteOutboundStatsBytesSent = 8u; constexpr uint64_t kRemoteOutboundStatsReportsCount = 9u; @@ -844,10 +845,10 @@ class RTCStatsCollectorTest : public ::testing::Test { // remote-outbound-rtp if (add_remote_outbound_stats) { graph.remote_outbound_rtp_id = "ROA4"; - media_info.receivers[0].last_sender_report_utc_timestamp_ms = - kRemoteOutboundStatsTimestampMs; - media_info.receivers[0].last_sender_report_remote_utc_timestamp_ms = - kRemoteOutboundStatsRemoteTimestampMs; + media_info.receivers[0].last_sender_report_utc_timestamp = + kRemoteOutboundStatsTimestamp; + media_info.receivers[0].last_sender_report_remote_utc_timestamp = + kRemoteOutboundStatsRemoteTimestamp; media_info.receivers[0].sender_reports_packets_sent = kRemoteOutboundStatsPacketsSent; media_info.receivers[0].sender_reports_bytes_sent = @@ -3533,10 +3534,9 @@ TEST_F(RTCStatsCollectorTest, RTCRemoteOutboundRtpAudioStreamStatsCollected) { const auto& remote_outbound_rtp = graph.full_report->Get(graph.remote_outbound_rtp_id) ->cast_to(); - EXPECT_EQ(remote_outbound_rtp.timestamp(), - Timestamp::Millis(kRemoteOutboundStatsTimestampMs)); + EXPECT_EQ(remote_outbound_rtp.timestamp(), kRemoteOutboundStatsTimestamp); EXPECT_FLOAT_EQ(*remote_outbound_rtp.remote_timestamp, - static_cast(kRemoteOutboundStatsRemoteTimestampMs)); + kRemoteOutboundStatsRemoteTimestamp.ms()); EXPECT_EQ(*remote_outbound_rtp.packets_sent, kRemoteOutboundStatsPacketsSent); EXPECT_EQ(*remote_outbound_rtp.bytes_sent, kRemoteOutboundStatsBytesSent); EXPECT_EQ(*remote_outbound_rtp.reports_sent, diff --git a/video/video_receive_stream2.cc b/video/video_receive_stream2.cc index eda4f91329..aa3ff4a6af 100644 --- a/video/video_receive_stream2.cc +++ b/video/video_receive_stream2.cc @@ -572,12 +572,12 @@ VideoReceiveStreamInterface::Stats VideoReceiveStream2::GetStats() const { std::optional rtcp_sr_stats = rtp_video_stream_receiver_.GetSenderReportStats(); if (rtcp_sr_stats) { - stats.last_sender_report_utc_timestamp_ms = - rtcp_sr_stats->last_arrival_ntp_timestamp.ToMs() - - rtc::kNtpJan1970Millisecs; - stats.last_sender_report_remote_utc_timestamp_ms = - rtcp_sr_stats->last_remote_ntp_timestamp.ToMs() - - rtc::kNtpJan1970Millisecs; + stats.last_sender_report_utc_timestamp = + Timestamp::Millis(rtcp_sr_stats->last_arrival_ntp_timestamp.ToMs() - + rtc::kNtpJan1970Millisecs); + stats.last_sender_report_remote_utc_timestamp = + Timestamp::Millis(rtcp_sr_stats->last_remote_ntp_timestamp.ToMs() - + rtc::kNtpJan1970Millisecs); stats.sender_reports_packets_sent = rtcp_sr_stats->packets_sent; stats.sender_reports_bytes_sent = rtcp_sr_stats->bytes_sent; stats.sender_reports_reports_count = rtcp_sr_stats->reports_count;