In RtpSenderVideo::UpdateConditionalRetransmit use typed time and framerate instead of plain ints
Bug: webrtc:13757
Change-Id: If2df5418dacd2b95387fa74a9bc226426b207aee
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/313041
Reviewed-by: Åsa Persson <asapersson@webrtc.org>
Commit-Queue: Danil Chapovalov <danilchap@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#40483}
diff --git a/modules/rtp_rtcp/BUILD.gn b/modules/rtp_rtcp/BUILD.gn
index 96c43bd..4a627e1 100644
--- a/modules/rtp_rtcp/BUILD.gn
+++ b/modules/rtp_rtcp/BUILD.gn
@@ -288,6 +288,7 @@
"../../api/transport/rtp:dependency_descriptor",
"../../api/transport/rtp:rtp_source",
"../../api/units:data_rate",
+ "../../api/units:frequency",
"../../api/units:time_delta",
"../../api/units:timestamp",
"../../api/video:encoded_frame",
@@ -313,6 +314,7 @@
"../../rtc_base:copy_on_write_buffer",
"../../rtc_base:divide_round",
"../../rtc_base:event_tracer",
+ "../../rtc_base:frequency_tracker",
"../../rtc_base:gtest_prod",
"../../rtc_base:logging",
"../../rtc_base:macromagic",
@@ -321,7 +323,6 @@
"../../rtc_base:race_checker",
"../../rtc_base:random",
"../../rtc_base:rate_limiter",
- "../../rtc_base:rate_statistics",
"../../rtc_base:rtc_numerics",
"../../rtc_base:safe_conversions",
"../../rtc_base:safe_minmax",
diff --git a/modules/rtp_rtcp/source/rtp_sender_video.cc b/modules/rtp_rtcp/source/rtp_sender_video.cc
index 41aff18..f29c5ea 100644
--- a/modules/rtp_rtcp/source/rtp_sender_video.cc
+++ b/modules/rtp_rtcp/source/rtp_sender_video.cc
@@ -24,6 +24,9 @@
#include "absl/strings/match.h"
#include "api/crypto/frame_encryptor_interface.h"
#include "api/transport/rtp/dependency_descriptor.h"
+#include "api/units/frequency.h"
+#include "api/units/time_delta.h"
+#include "api/units/timestamp.h"
#include "modules/remote_bitrate_estimator/test/bwe_test_logging.h"
#include "modules/rtp_rtcp/include/rtp_rtcp_defines.h"
#include "modules/rtp_rtcp/source/absolute_capture_time_sender.h"
@@ -45,7 +48,8 @@
namespace {
constexpr size_t kRedForFecHeaderLength = 1;
-constexpr int64_t kMaxUnretransmittableFrameIntervalMs = 33 * 4;
+constexpr TimeDelta kMaxUnretransmittableFrameInterval =
+ TimeDelta::Millis(33 * 4);
void BuildRedPayload(const RtpPacketToSend& media_packet,
RtpPacketToSend* red_packet) {
@@ -781,8 +785,7 @@
MutexLock lock(&stats_mutex_);
// Media packet storage.
if ((retransmission_settings & kConditionallyRetransmitHigherLayers) &&
- UpdateConditionalRetransmit(temporal_id,
- expected_retransmission_time.ms())) {
+ UpdateConditionalRetransmit(temporal_id, expected_retransmission_time)) {
retransmission_settings |= kRetransmitHigherLayers;
}
@@ -815,39 +818,37 @@
bool RTPSenderVideo::UpdateConditionalRetransmit(
uint8_t temporal_id,
- int64_t expected_retransmission_time_ms) {
- int64_t now_ms = clock_->TimeInMilliseconds();
+ TimeDelta expected_retransmission_time) {
+ Timestamp now = clock_->CurrentTime();
// Update stats for any temporal layer.
TemporalLayerStats* current_layer_stats =
&frame_stats_by_temporal_layer_[temporal_id];
- current_layer_stats->frame_rate_fp1000s.Update(1, now_ms);
- int64_t tl_frame_interval = now_ms - current_layer_stats->last_frame_time_ms;
- current_layer_stats->last_frame_time_ms = now_ms;
+ current_layer_stats->frame_rate.Update(now);
+ TimeDelta tl_frame_interval = now - current_layer_stats->last_frame_time;
+ current_layer_stats->last_frame_time = now;
// Conditional retransmit only applies to upper layers.
if (temporal_id != kNoTemporalIdx && temporal_id > 0) {
- if (tl_frame_interval >= kMaxUnretransmittableFrameIntervalMs) {
+ if (tl_frame_interval >= kMaxUnretransmittableFrameInterval) {
// Too long since a retransmittable frame in this layer, enable NACK
// protection.
return true;
} else {
// Estimate when the next frame of any lower layer will be sent.
- const int64_t kUndefined = std::numeric_limits<int64_t>::max();
- int64_t expected_next_frame_time = kUndefined;
+ Timestamp expected_next_frame_time = Timestamp::PlusInfinity();
for (int i = temporal_id - 1; i >= 0; --i) {
TemporalLayerStats* stats = &frame_stats_by_temporal_layer_[i];
- absl::optional<uint32_t> rate = stats->frame_rate_fp1000s.Rate(now_ms);
- if (rate) {
- int64_t tl_next = stats->last_frame_time_ms + 1000000 / *rate;
- if (tl_next - now_ms > -expected_retransmission_time_ms &&
+ absl::optional<Frequency> rate = stats->frame_rate.Rate(now);
+ if (rate > Frequency::Zero()) {
+ Timestamp tl_next = stats->last_frame_time + 1 / *rate;
+ if (tl_next - now > -expected_retransmission_time &&
tl_next < expected_next_frame_time) {
expected_next_frame_time = tl_next;
}
}
}
- if (expected_next_frame_time == kUndefined ||
- expected_next_frame_time - now_ms > expected_retransmission_time_ms) {
+ if (expected_next_frame_time - now > expected_retransmission_time) {
// The next frame in a lower layer is expected at a later time (or
// unable to tell due to lack of data) than a retransmission is
// estimated to be able to arrive, so allow this packet to be nacked.
diff --git a/modules/rtp_rtcp/source/rtp_sender_video.h b/modules/rtp_rtcp/source/rtp_sender_video.h
index a9edc2d..acf4d94 100644
--- a/modules/rtp_rtcp/source/rtp_sender_video.h
+++ b/modules/rtp_rtcp/source/rtp_sender_video.h
@@ -24,6 +24,8 @@
#include "api/task_queue/task_queue_base.h"
#include "api/task_queue/task_queue_factory.h"
#include "api/transport/rtp/dependency_descriptor.h"
+#include "api/units/time_delta.h"
+#include "api/units/timestamp.h"
#include "api/video/video_codec_type.h"
#include "api/video/video_frame_type.h"
#include "api/video/video_layers_allocation.h"
@@ -36,9 +38,9 @@
#include "modules/rtp_rtcp/source/rtp_video_header.h"
#include "modules/rtp_rtcp/source/video_fec_generator.h"
#include "rtc_base/bitrate_tracker.h"
+#include "rtc_base/frequency_tracker.h"
#include "rtc_base/one_time_event.h"
#include "rtc_base/race_checker.h"
-#include "rtc_base/rate_statistics.h"
#include "rtc_base/synchronization/mutex.h"
#include "rtc_base/thread_annotations.h"
@@ -62,7 +64,7 @@
class RTPSenderVideo : public RTPVideoFrameSenderInterface {
public:
- static constexpr int64_t kTLRateWindowSizeMs = 2500;
+ static constexpr TimeDelta kTLRateWindowSize = TimeDelta::Millis(2'500);
struct Config {
Config() = default;
@@ -154,14 +156,8 @@
private:
struct TemporalLayerStats {
- TemporalLayerStats()
- : frame_rate_fp1000s(kTLRateWindowSizeMs, 1000 * 1000),
- last_frame_time_ms(0) {}
- // Frame rate, in frames per 1000 seconds. This essentially turns the fps
- // value into a fixed point value with three decimals. Improves precision at
- // low frame rates.
- RateStatistics frame_rate_fp1000s;
- int64_t last_frame_time_ms;
+ FrequencyTracker frame_rate{kTLRateWindowSize};
+ Timestamp last_frame_time = Timestamp::Zero();
};
enum class SendVideoLayersAllocation {
@@ -189,7 +185,7 @@
bool red_enabled() const { return red_payload_type_.has_value(); }
bool UpdateConditionalRetransmit(uint8_t temporal_id,
- int64_t expected_retransmission_time_ms)
+ TimeDelta expected_retransmission_time)
RTC_EXCLUSIVE_LOCKS_REQUIRED(stats_mutex_);
void MaybeUpdateCurrentPlayoutDelay(const RTPVideoHeader& header)
diff --git a/modules/rtp_rtcp/source/rtp_sender_video_unittest.cc b/modules/rtp_rtcp/source/rtp_sender_video_unittest.cc
index 8844b62..7ce96e0 100644
--- a/modules/rtp_rtcp/source/rtp_sender_video_unittest.cc
+++ b/modules/rtp_rtcp/source/rtp_sender_video_unittest.cc
@@ -414,8 +414,7 @@
// Fill averaging window to prevent rounding errors.
constexpr int kNumRepetitions =
- (RTPSenderVideo::kTLRateWindowSizeMs + (kFrameInterval.ms() / 2)) /
- kFrameInterval.ms();
+ RTPSenderVideo::kTLRateWindowSize / kFrameInterval;
constexpr int kPattern[] = {0, 2, 1, 2};
auto& vp8_header = header.video_type_header.emplace<RTPVideoHeaderVP8>();
for (size_t i = 0; i < arraysize(kPattern) * kNumRepetitions; ++i) {
@@ -466,8 +465,7 @@
// Fill averaging window to prevent rounding errors.
constexpr int kNumRepetitions =
- (RTPSenderVideo::kTLRateWindowSizeMs + (kFrameInterval.ms() / 2)) /
- kFrameInterval.ms();
+ RTPSenderVideo::kTLRateWindowSize / kFrameInterval;
constexpr int kPattern[] = {0, 2, 2, 2};
auto& vp8_header = header.video_type_header.emplace<RTPVideoHeaderVP8>();
for (size_t i = 0; i < arraysize(kPattern) * kNumRepetitions; ++i) {
