In RtpSource represent time with Timestamp type instead of int64_t
Bug: webrtc:13757
Change-Id: I5d7da9c9aee489e4b57d361de174c59713cb2b14
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/317780
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Danil Chapovalov <danilchap@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#40650}
diff --git a/api/transport/rtp/BUILD.gn b/api/transport/rtp/BUILD.gn
index 205bbcc..6f2a15e 100644
--- a/api/transport/rtp/BUILD.gn
+++ b/api/transport/rtp/BUILD.gn
@@ -14,6 +14,7 @@
deps = [
"../../../api:rtp_headers",
"../../../api/units:time_delta",
+ "../../../api/units:timestamp",
"../../../rtc_base:checks",
]
absl_deps = [ "//third_party/abseil-cpp/absl/types:optional" ]
diff --git a/api/transport/rtp/rtp_source.h b/api/transport/rtp/rtp_source.h
index e51dcd7..41a0552 100644
--- a/api/transport/rtp/rtp_source.h
+++ b/api/transport/rtp/rtp_source.h
@@ -16,6 +16,7 @@
#include "absl/types/optional.h"
#include "api/rtp_headers.h"
#include "api/units/time_delta.h"
+#include "api/units/timestamp.h"
#include "rtc_base/checks.h"
namespace webrtc {
@@ -44,12 +45,25 @@
RtpSource() = delete;
+ RtpSource(Timestamp timestamp,
+ uint32_t source_id,
+ RtpSourceType source_type,
+ uint32_t rtp_timestamp,
+ const RtpSource::Extensions& extensions)
+ : timestamp_(timestamp),
+ source_id_(source_id),
+ source_type_(source_type),
+ extensions_(extensions),
+ rtp_timestamp_(rtp_timestamp) {}
+
+ // TODO(bugs.webrtc.org/13757): deprecate when chromium stop using this
+ // and remove after 2023-09-18
RtpSource(int64_t timestamp_ms,
uint32_t source_id,
RtpSourceType source_type,
uint32_t rtp_timestamp,
const RtpSource::Extensions& extensions)
- : timestamp_ms_(timestamp_ms),
+ : timestamp_(Timestamp::Millis(timestamp_ms)),
source_id_(source_id),
source_type_(source_type),
extensions_(extensions),
@@ -59,10 +73,14 @@
RtpSource& operator=(const RtpSource&) = default;
~RtpSource() = default;
- int64_t timestamp_ms() const { return timestamp_ms_; }
- void update_timestamp_ms(int64_t timestamp_ms) {
- RTC_DCHECK_LE(timestamp_ms_, timestamp_ms);
- timestamp_ms_ = timestamp_ms;
+ Timestamp timestamp() const { return timestamp_; }
+
+ // TODO(bugs.webrtc.org/13757): deprecate when chromium stop using this
+ // and remove after 2023-09-18
+ int64_t timestamp_ms() const { return timestamp_.ms(); }
+ [[deprecated]] void update_timestamp_ms(int64_t timestamp_ms) {
+ RTC_DCHECK_LE(timestamp_.ms(), timestamp_ms);
+ timestamp_ = Timestamp::Millis(timestamp_ms);
}
// The identifier of the source can be the CSRC or the SSRC.
@@ -90,7 +108,7 @@
}
bool operator==(const RtpSource& o) const {
- return timestamp_ms_ == o.timestamp_ms() && source_id_ == o.source_id() &&
+ return timestamp_ == o.timestamp() && source_id_ == o.source_id() &&
source_type_ == o.source_type() &&
extensions_.audio_level == o.extensions_.audio_level &&
extensions_.absolute_capture_time ==
@@ -99,7 +117,7 @@
}
private:
- int64_t timestamp_ms_;
+ Timestamp timestamp_;
uint32_t source_id_;
RtpSourceType source_type_;
RtpSource::Extensions extensions_;
diff --git a/media/engine/webrtc_video_engine_unittest.cc b/media/engine/webrtc_video_engine_unittest.cc
index 7719dc2..8875883 100644
--- a/media/engine/webrtc_video_engine_unittest.cc
+++ b/media/engine/webrtc_video_engine_unittest.cc
@@ -9767,7 +9767,7 @@
EXPECT_EQ(source.source_id(), kSsrc);
EXPECT_EQ(source.source_type(), webrtc::RtpSourceType::SSRC);
int64_t rtp_timestamp_1 = source.rtp_timestamp();
- int64_t timestamp_ms_1 = source.timestamp_ms();
+ Timestamp timestamp_1 = source.timestamp();
// Send and receive another frame.
SendFrame();
@@ -9781,10 +9781,10 @@
EXPECT_EQ(source.source_id(), kSsrc);
EXPECT_EQ(source.source_type(), webrtc::RtpSourceType::SSRC);
int64_t rtp_timestamp_2 = source.rtp_timestamp();
- int64_t timestamp_ms_2 = source.timestamp_ms();
+ Timestamp timestamp_2 = source.timestamp();
EXPECT_GT(rtp_timestamp_2, rtp_timestamp_1);
- EXPECT_GT(timestamp_ms_2, timestamp_ms_1);
+ EXPECT_GT(timestamp_2, timestamp_1);
}
TEST_F(WebRtcVideoChannelTest, SetsRidsOnSendStream) {
diff --git a/modules/rtp_rtcp/source/source_tracker.cc b/modules/rtp_rtcp/source/source_tracker.cc
index 13d848d..51e8f1c 100644
--- a/modules/rtp_rtcp/source/source_tracker.cc
+++ b/modules/rtp_rtcp/source/source_tracker.cc
@@ -79,7 +79,7 @@
const SourceEntry& entry = pair.second;
sources.emplace_back(
- entry.timestamp.ms(), key.source, key.source_type, entry.rtp_timestamp,
+ entry.timestamp, key.source, key.source_type, entry.rtp_timestamp,
RtpSource::Extensions{
.audio_level = entry.audio_level,
.absolute_capture_time = entry.absolute_capture_time,
diff --git a/modules/rtp_rtcp/source/source_tracker_unittest.cc b/modules/rtp_rtcp/source/source_tracker_unittest.cc
index c11142b..e14a389 100644
--- a/modules/rtp_rtcp/source/source_tracker_unittest.cc
+++ b/modules/rtp_rtcp/source/source_tracker_unittest.cc
@@ -46,7 +46,7 @@
explicit ExpectedSourceTracker(Clock* clock) : clock_(clock) {}
void OnFrameDelivered(const RtpPacketInfos& packet_infos) {
- const int64_t now_ms = clock_->TimeInMilliseconds();
+ const Timestamp now = clock_->CurrentTime();
for (const auto& packet_info : packet_infos) {
RtpSource::Extensions extensions = {
@@ -54,26 +54,26 @@
packet_info.local_capture_clock_offset()};
for (const auto& csrc : packet_info.csrcs()) {
- entries_.emplace_front(now_ms, csrc, RtpSourceType::CSRC,
+ entries_.emplace_front(now, csrc, RtpSourceType::CSRC,
packet_info.rtp_timestamp(), extensions);
}
- entries_.emplace_front(now_ms, packet_info.ssrc(), RtpSourceType::SSRC,
+ entries_.emplace_front(now, packet_info.ssrc(), RtpSourceType::SSRC,
packet_info.rtp_timestamp(), extensions);
}
- PruneEntries(now_ms);
+ PruneEntries(now);
}
std::vector<RtpSource> GetSources() const {
- PruneEntries(clock_->TimeInMilliseconds());
+ PruneEntries(clock_->CurrentTime());
return std::vector<RtpSource>(entries_.begin(), entries_.end());
}
private:
- void PruneEntries(int64_t now_ms) const {
- const int64_t prune_ms = now_ms - 10000; // 10 seconds
+ void PruneEntries(Timestamp now) const {
+ const Timestamp prune = now - TimeDelta::Seconds(10);
std::set<std::pair<RtpSourceType, uint32_t>> seen;
@@ -84,7 +84,7 @@
++next;
auto key = std::make_pair(it->source_type(), it->source_id());
- if (!seen.insert(key).second || it->timestamp_ms() < prune_ms) {
+ if (!seen.insert(key).second || it->timestamp() < prune) {
entries_.erase(it);
}
@@ -297,18 +297,17 @@
time_controller.AdvanceTime(TimeDelta::Zero());
- EXPECT_THAT(
- tracker.GetSources(),
- ElementsAre(RtpSource(timestamp.ms(), kSsrc2, RtpSourceType::SSRC,
- kRtpTimestamp1, extensions1),
- RtpSource(timestamp.ms(), kCsrcs2, RtpSourceType::CSRC,
- kRtpTimestamp1, extensions1),
- RtpSource(timestamp.ms(), kSsrc1, RtpSourceType::SSRC,
- kRtpTimestamp0, extensions0),
- RtpSource(timestamp.ms(), kCsrcs1, RtpSourceType::CSRC,
- kRtpTimestamp0, extensions0),
- RtpSource(timestamp.ms(), kCsrcs0, RtpSourceType::CSRC,
- kRtpTimestamp0, extensions0)));
+ EXPECT_THAT(tracker.GetSources(),
+ ElementsAre(RtpSource(timestamp, kSsrc2, RtpSourceType::SSRC,
+ kRtpTimestamp1, extensions1),
+ RtpSource(timestamp, kCsrcs2, RtpSourceType::CSRC,
+ kRtpTimestamp1, extensions1),
+ RtpSource(timestamp, kSsrc1, RtpSourceType::SSRC,
+ kRtpTimestamp0, extensions0),
+ RtpSource(timestamp, kCsrcs1, RtpSourceType::CSRC,
+ kRtpTimestamp0, extensions0),
+ RtpSource(timestamp, kCsrcs0, RtpSourceType::CSRC,
+ kRtpTimestamp0, extensions0)));
}
TEST(SourceTrackerTest, OnFrameDeliveredRecordsSourcesSameSsrc) {
@@ -363,16 +362,15 @@
.absolute_capture_time = kAbsoluteCaptureTime,
.local_capture_clock_offset = kLocalCaptureClockOffset};
- EXPECT_THAT(
- tracker.GetSources(),
- ElementsAre(RtpSource(timestamp.ms(), kSsrc, RtpSourceType::SSRC,
- kRtpTimestamp2, extensions2),
- RtpSource(timestamp.ms(), kCsrcs0, RtpSourceType::CSRC,
- kRtpTimestamp2, extensions2),
- RtpSource(timestamp.ms(), kCsrcs2, RtpSourceType::CSRC,
- kRtpTimestamp1, extensions1),
- RtpSource(timestamp.ms(), kCsrcs1, RtpSourceType::CSRC,
- kRtpTimestamp0, extensions0)));
+ EXPECT_THAT(tracker.GetSources(),
+ ElementsAre(RtpSource(timestamp, kSsrc, RtpSourceType::SSRC,
+ kRtpTimestamp2, extensions2),
+ RtpSource(timestamp, kCsrcs0, RtpSourceType::CSRC,
+ kRtpTimestamp2, extensions2),
+ RtpSource(timestamp, kCsrcs2, RtpSourceType::CSRC,
+ kRtpTimestamp1, extensions1),
+ RtpSource(timestamp, kCsrcs1, RtpSourceType::CSRC,
+ kRtpTimestamp0, extensions0)));
}
TEST(SourceTrackerTest, OnFrameDeliveredUpdatesSources) {
@@ -427,14 +425,13 @@
time_controller.AdvanceTime(TimeDelta::Zero());
Timestamp timestamp_0 = time_controller.GetClock()->CurrentTime();
- EXPECT_THAT(
- tracker.GetSources(),
- ElementsAre(RtpSource(timestamp_0.ms(), kSsrc1, RtpSourceType::SSRC,
- kRtpTimestamp0, extensions0),
- RtpSource(timestamp_0.ms(), kCsrcs1, RtpSourceType::CSRC,
- kRtpTimestamp0, extensions0),
- RtpSource(timestamp_0.ms(), kCsrcs0, RtpSourceType::CSRC,
- kRtpTimestamp0, extensions0)));
+ EXPECT_THAT(tracker.GetSources(),
+ ElementsAre(RtpSource(timestamp_0, kSsrc1, RtpSourceType::SSRC,
+ kRtpTimestamp0, extensions0),
+ RtpSource(timestamp_0, kCsrcs1, RtpSourceType::CSRC,
+ kRtpTimestamp0, extensions0),
+ RtpSource(timestamp_0, kCsrcs0, RtpSourceType::CSRC,
+ kRtpTimestamp0, extensions0)));
// Deliver packets with updated sources.
@@ -448,16 +445,15 @@
time_controller.AdvanceTime(TimeDelta::Zero());
Timestamp timestamp_1 = time_controller.GetClock()->CurrentTime();
- EXPECT_THAT(
- tracker.GetSources(),
- ElementsAre(RtpSource(timestamp_1.ms(), kSsrc1, RtpSourceType::SSRC,
- kRtpTimestamp1, extensions1),
- RtpSource(timestamp_1.ms(), kCsrcs2, RtpSourceType::CSRC,
- kRtpTimestamp1, extensions1),
- RtpSource(timestamp_1.ms(), kCsrcs0, RtpSourceType::CSRC,
- kRtpTimestamp1, extensions1),
- RtpSource(timestamp_0.ms(), kCsrcs1, RtpSourceType::CSRC,
- kRtpTimestamp0, extensions0)));
+ EXPECT_THAT(tracker.GetSources(),
+ ElementsAre(RtpSource(timestamp_1, kSsrc1, RtpSourceType::SSRC,
+ kRtpTimestamp1, extensions1),
+ RtpSource(timestamp_1, kCsrcs2, RtpSourceType::CSRC,
+ kRtpTimestamp1, extensions1),
+ RtpSource(timestamp_1, kCsrcs0, RtpSourceType::CSRC,
+ kRtpTimestamp1, extensions1),
+ RtpSource(timestamp_0, kCsrcs1, RtpSourceType::CSRC,
+ kRtpTimestamp0, extensions0)));
// Deliver more packets with update csrcs and a new ssrc.
time_controller.AdvanceTime(TimeDelta::Millis(17));
@@ -471,18 +467,17 @@
time_controller.AdvanceTime(TimeDelta::Zero());
Timestamp timestamp_2 = time_controller.GetClock()->CurrentTime();
- EXPECT_THAT(
- tracker.GetSources(),
- ElementsAre(RtpSource(timestamp_2.ms(), kSsrc2, RtpSourceType::SSRC,
- kRtpTimestamp2, extensions2),
- RtpSource(timestamp_2.ms(), kCsrcs0, RtpSourceType::CSRC,
- kRtpTimestamp2, extensions2),
- RtpSource(timestamp_1.ms(), kSsrc1, RtpSourceType::SSRC,
- kRtpTimestamp1, extensions1),
- RtpSource(timestamp_1.ms(), kCsrcs2, RtpSourceType::CSRC,
- kRtpTimestamp1, extensions1),
- RtpSource(timestamp_0.ms(), kCsrcs1, RtpSourceType::CSRC,
- kRtpTimestamp0, extensions0)));
+ EXPECT_THAT(tracker.GetSources(),
+ ElementsAre(RtpSource(timestamp_2, kSsrc2, RtpSourceType::SSRC,
+ kRtpTimestamp2, extensions2),
+ RtpSource(timestamp_2, kCsrcs0, RtpSourceType::CSRC,
+ kRtpTimestamp2, extensions2),
+ RtpSource(timestamp_1, kSsrc1, RtpSourceType::SSRC,
+ kRtpTimestamp1, extensions1),
+ RtpSource(timestamp_1, kCsrcs2, RtpSourceType::CSRC,
+ kRtpTimestamp1, extensions1),
+ RtpSource(timestamp_0, kCsrcs1, RtpSourceType::CSRC,
+ kRtpTimestamp0, extensions0)));
}
TEST(SourceTrackerTest, TimedOutSourcesAreRemoved) {
@@ -531,14 +526,13 @@
.absolute_capture_time = kAbsoluteCaptureTime1,
.local_capture_clock_offset = kLocalCaptureClockOffset1};
- EXPECT_THAT(
- tracker.GetSources(),
- ElementsAre(RtpSource(timestamp_1.ms(), kSsrc, RtpSourceType::SSRC,
- kRtpTimestamp1, extensions1),
- RtpSource(timestamp_1.ms(), kCsrcs2, RtpSourceType::CSRC,
- kRtpTimestamp1, extensions1),
- RtpSource(timestamp_1.ms(), kCsrcs0, RtpSourceType::CSRC,
- kRtpTimestamp1, extensions1)));
+ EXPECT_THAT(tracker.GetSources(),
+ ElementsAre(RtpSource(timestamp_1, kSsrc, RtpSourceType::SSRC,
+ kRtpTimestamp1, extensions1),
+ RtpSource(timestamp_1, kCsrcs2, RtpSourceType::CSRC,
+ kRtpTimestamp1, extensions1),
+ RtpSource(timestamp_1, kCsrcs0, RtpSourceType::CSRC,
+ kRtpTimestamp1, extensions1)));
}
} // namespace webrtc
diff --git a/video/video_receive_stream2_unittest.cc b/video/video_receive_stream2_unittest.cc
index 4476c46..7ae446f 100644
--- a/video/video_receive_stream2_unittest.cc
+++ b/video/video_receive_stream2_unittest.cc
@@ -563,12 +563,12 @@
EXPECT_THAT(video_receive_stream_->GetSources(), IsEmpty());
// Render one video frame.
- int64_t timestamp_ms_min = clock_->TimeInMilliseconds();
+ Timestamp timestamp_min = clock_->CurrentTime();
video_receive_stream_->OnCompleteFrame(std::move(test_frame));
// Verify that the per-packet information is passed to the renderer.
EXPECT_THAT(fake_renderer_.WaitForFrame(kDefaultTimeOut),
RenderedFrameWith(PacketInfos(ElementsAreArray(packet_infos))));
- int64_t timestamp_ms_max = clock_->TimeInMilliseconds();
+ Timestamp timestamp_max = clock_->CurrentTime();
// Verify that the per-packet information also updates `GetSources()`.
std::vector<RtpSource> sources = video_receive_stream_->GetSources();
@@ -583,8 +583,8 @@
EXPECT_EQ(it->source_id(), kSsrc);
EXPECT_EQ(it->source_type(), RtpSourceType::SSRC);
EXPECT_EQ(it->rtp_timestamp(), kRtpTimestamp);
- EXPECT_GE(it->timestamp_ms(), timestamp_ms_min);
- EXPECT_LE(it->timestamp_ms(), timestamp_ms_max);
+ EXPECT_GE(it->timestamp(), timestamp_min);
+ EXPECT_LE(it->timestamp(), timestamp_max);
}
{
auto it = std::find_if(sources.begin(), sources.end(),
@@ -596,8 +596,8 @@
EXPECT_EQ(it->source_id(), kCsrc);
EXPECT_EQ(it->source_type(), RtpSourceType::CSRC);
EXPECT_EQ(it->rtp_timestamp(), kRtpTimestamp);
- EXPECT_GE(it->timestamp_ms(), timestamp_ms_min);
- EXPECT_LE(it->timestamp_ms(), timestamp_ms_max);
+ EXPECT_GE(it->timestamp(), timestamp_min);
+ EXPECT_LE(it->timestamp(), timestamp_max);
}
}
