From 47ed99872d8b6392d2eb2e3c890b0eb00209fa20 Mon Sep 17 00:00:00 2001 From: Paul Hallak Date: Fri, 21 May 2021 10:34:34 +0200 Subject: [PATCH] Use the clock to convert absolute capture timestamps to NTP times. This allows callers to use timestamps generated from their own clocks without worrying about converting to webrtc time. No-Try because of lack of infra lack of capacity on macs. No-Try: True Bug: webrtc:11327 Change-Id: I7b1935654a2b23cf844c7b3622ed68763ced9da5 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/219785 Commit-Queue: Paul Hallak Reviewed-by: Danil Chapovalov Reviewed-by: Minyue Li Cr-Commit-Position: refs/heads/master@{#34076} --- modules/rtp_rtcp/source/rtp_sender_audio.cc | 3 ++- modules/rtp_rtcp/source/rtp_sender_audio.h | 2 ++ .../rtp_rtcp/source/rtp_sender_audio_unittest.cc | 13 ++++++++----- modules/rtp_rtcp/source/rtp_sender_video.cc | 3 ++- modules/rtp_rtcp/source/rtp_sender_video.h | 1 + .../rtp_rtcp/source/rtp_sender_video_unittest.cc | 13 ++++++++----- 6 files changed, 23 insertions(+), 12 deletions(-) diff --git a/modules/rtp_rtcp/source/rtp_sender_audio.cc b/modules/rtp_rtcp/source/rtp_sender_audio.cc index cfb89c1ba0..4d72211b7c 100644 --- a/modules/rtp_rtcp/source/rtp_sender_audio.cc +++ b/modules/rtp_rtcp/source/rtp_sender_audio.cc @@ -287,7 +287,8 @@ bool RTPSenderAudio::SendAudio(AudioFrameType frame_type, // Replace missing value with 0 (invalid frequency), this will trigger // absolute capture time sending. encoder_rtp_timestamp_frequency.value_or(0), - Int64MsToUQ32x32(absolute_capture_timestamp_ms + NtpOffsetMs()), + Int64MsToUQ32x32(clock_->ConvertTimestampToNtpTimeInMilliseconds( + absolute_capture_timestamp_ms)), /*estimated_capture_clock_offset=*/ include_capture_clock_offset_ ? absl::make_optional(0) : absl::nullopt); if (absolute_capture_time) { diff --git a/modules/rtp_rtcp/source/rtp_sender_audio.h b/modules/rtp_rtcp/source/rtp_sender_audio.h index 57b9dd7ce6..6d61facc9a 100644 --- a/modules/rtp_rtcp/source/rtp_sender_audio.h +++ b/modules/rtp_rtcp/source/rtp_sender_audio.h @@ -51,6 +51,8 @@ class RTPSenderAudio { const uint8_t* payload_data, size_t payload_size); + // `absolute_capture_timestamp_ms` and `Clock::CurrentTime` + // should be using the same epoch. bool SendAudio(AudioFrameType frame_type, int8_t payload_type, uint32_t rtp_timestamp, diff --git a/modules/rtp_rtcp/source/rtp_sender_audio_unittest.cc b/modules/rtp_rtcp/source/rtp_sender_audio_unittest.cc index d75f4e8947..0221800ea8 100644 --- a/modules/rtp_rtcp/source/rtp_sender_audio_unittest.cc +++ b/modules/rtp_rtcp/source/rtp_sender_audio_unittest.cc @@ -19,7 +19,6 @@ #include "modules/rtp_rtcp/source/rtp_header_extensions.h" #include "modules/rtp_rtcp/source/rtp_packet_received.h" #include "modules/rtp_rtcp/source/rtp_rtcp_impl2.h" -#include "modules/rtp_rtcp/source/time_util.h" #include "test/field_trial.h" #include "test/gmock.h" #include "test/gtest.h" @@ -167,8 +166,10 @@ TEST_F(RtpSenderAudioTest, SendAudioWithAbsoluteCaptureTime) { transport_.last_sent_packet() .GetExtension(); EXPECT_TRUE(absolute_capture_time); - EXPECT_EQ(absolute_capture_time->absolute_capture_timestamp, - Int64MsToUQ32x32(kAbsoluteCaptureTimestampMs + NtpOffsetMs())); + EXPECT_EQ( + absolute_capture_time->absolute_capture_timestamp, + Int64MsToUQ32x32(fake_clock_.ConvertTimestampToNtpTimeInMilliseconds( + kAbsoluteCaptureTimestampMs))); EXPECT_FALSE( absolute_capture_time->estimated_capture_clock_offset.has_value()); } @@ -201,8 +202,10 @@ TEST_F(RtpSenderAudioTest, transport_.last_sent_packet() .GetExtension(); EXPECT_TRUE(absolute_capture_time); - EXPECT_EQ(absolute_capture_time->absolute_capture_timestamp, - Int64MsToUQ32x32(kAbsoluteCaptureTimestampMs + NtpOffsetMs())); + EXPECT_EQ( + absolute_capture_time->absolute_capture_timestamp, + Int64MsToUQ32x32(fake_clock_.ConvertTimestampToNtpTimeInMilliseconds( + kAbsoluteCaptureTimestampMs))); EXPECT_TRUE( absolute_capture_time->estimated_capture_clock_offset.has_value()); EXPECT_EQ(0, *absolute_capture_time->estimated_capture_clock_offset); diff --git a/modules/rtp_rtcp/source/rtp_sender_video.cc b/modules/rtp_rtcp/source/rtp_sender_video.cc index 6e620777c6..d344695fdf 100644 --- a/modules/rtp_rtcp/source/rtp_sender_video.cc +++ b/modules/rtp_rtcp/source/rtp_sender_video.cc @@ -523,7 +523,8 @@ bool RTPSenderVideo::SendVideo( AbsoluteCaptureTimeSender::GetSource(single_packet->Ssrc(), single_packet->Csrcs()), single_packet->Timestamp(), kVideoPayloadTypeFrequency, - Int64MsToUQ32x32(single_packet->capture_time_ms() + NtpOffsetMs()), + Int64MsToUQ32x32( + clock_->ConvertTimestampToNtpTimeInMilliseconds(capture_time_ms)), /*estimated_capture_clock_offset=*/ include_capture_clock_offset_ ? estimated_capture_clock_offset_ms : absl::nullopt); diff --git a/modules/rtp_rtcp/source/rtp_sender_video.h b/modules/rtp_rtcp/source/rtp_sender_video.h index 06f3d20014..ba8d7e8360 100644 --- a/modules/rtp_rtcp/source/rtp_sender_video.h +++ b/modules/rtp_rtcp/source/rtp_sender_video.h @@ -89,6 +89,7 @@ class RTPSenderVideo { virtual ~RTPSenderVideo(); // expected_retransmission_time_ms.has_value() -> retransmission allowed. + // `capture_time_ms` and `clock::CurrentTime` should be using the same epoch. // Calls to this method is assumed to be externally serialized. // |estimated_capture_clock_offset_ms| is an estimated clock offset between // this sender and the original capturer, for this video packet. See diff --git a/modules/rtp_rtcp/source/rtp_sender_video_unittest.cc b/modules/rtp_rtcp/source/rtp_sender_video_unittest.cc index 508d70f8e3..ea727828cc 100644 --- a/modules/rtp_rtcp/source/rtp_sender_video_unittest.cc +++ b/modules/rtp_rtcp/source/rtp_sender_video_unittest.cc @@ -34,7 +34,6 @@ #include "modules/rtp_rtcp/source/rtp_packet_received.h" #include "modules/rtp_rtcp/source/rtp_rtcp_impl2.h" #include "modules/rtp_rtcp/source/rtp_video_layers_allocation_extension.h" -#include "modules/rtp_rtcp/source/time_util.h" #include "rtc_base/arraysize.h" #include "rtc_base/rate_limiter.h" #include "rtc_base/task_queue_for_test.h" @@ -1054,8 +1053,10 @@ TEST_P(RtpSenderVideoTest, AbsoluteCaptureTime) { packet.GetExtension(); if (absolute_capture_time) { ++packets_with_abs_capture_time; - EXPECT_EQ(absolute_capture_time->absolute_capture_timestamp, - Int64MsToUQ32x32(kAbsoluteCaptureTimestampMs + NtpOffsetMs())); + EXPECT_EQ( + absolute_capture_time->absolute_capture_timestamp, + Int64MsToUQ32x32(fake_clock_.ConvertTimestampToNtpTimeInMilliseconds( + kAbsoluteCaptureTimestampMs))); EXPECT_FALSE( absolute_capture_time->estimated_capture_clock_offset.has_value()); } @@ -1092,8 +1093,10 @@ TEST_P(RtpSenderVideoTest, AbsoluteCaptureTimeWithCaptureClockOffset) { packet.GetExtension(); if (absolute_capture_time) { ++packets_with_abs_capture_time; - EXPECT_EQ(absolute_capture_time->absolute_capture_timestamp, - Int64MsToUQ32x32(kAbsoluteCaptureTimestampMs + NtpOffsetMs())); + EXPECT_EQ( + absolute_capture_time->absolute_capture_timestamp, + Int64MsToUQ32x32(fake_clock_.ConvertTimestampToNtpTimeInMilliseconds( + kAbsoluteCaptureTimestampMs))); EXPECT_EQ(kExpectedCaptureClockOffset, absolute_capture_time->estimated_capture_clock_offset); }