From e9126c18bfc98041bec8e7e1017552565f7b5bc4 Mon Sep 17 00:00:00 2001 From: Evan Shrubsole Date: Mon, 7 Mar 2022 14:50:51 +0100 Subject: [PATCH] Migrate VCMInterFrameDelay to use Time units MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Additionally, * Moved to its own GN target. * Added unittests. * Removed unused variable `_zeroWallClock`. * Renamed variables to match style guide. * Moved fields _dTS and _wrapArounds to variables. Change-Id: I7aa8b8dec55abab49ceabe838dabf2a7e13d685d Bug: webrtc:13756 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/253580 Reviewed-by: Niels Moller Reviewed-by: Erik Språng Reviewed-by: Harald Alvestrand Commit-Queue: Evan Shrubsole Cr-Commit-Position: refs/heads/main@{#36147} --- api/video/encoded_frame.cc | 6 + api/video/encoded_frame.h | 3 + modules/video_coding/BUILD.gn | 21 +- modules/video_coding/frame_buffer2.cc | 10 +- modules/video_coding/inter_frame_delay.cc | 91 ++++----- modules/video_coding/inter_frame_delay.h | 40 ++-- .../inter_frame_delay_unittest.cc | 190 ++++++++++++++++++ modules/video_coding/jitter_buffer.cc | 25 ++- video/BUILD.gn | 1 + video/frame_buffer_proxy.cc | 33 +-- 10 files changed, 298 insertions(+), 122 deletions(-) create mode 100644 modules/video_coding/inter_frame_delay_unittest.cc diff --git a/api/video/encoded_frame.cc b/api/video/encoded_frame.cc index 56d199f30f..c5e2abbbb4 100644 --- a/api/video/encoded_frame.cc +++ b/api/video/encoded_frame.cc @@ -14,6 +14,12 @@ namespace webrtc { +absl::optional EncodedFrame::ReceivedTimestamp() const { + return ReceivedTime() >= 0 + ? absl::make_optional(Timestamp::Millis(ReceivedTime())) + : absl::nullopt; +} + absl::optional EncodedFrame::RenderTimestamp() const { return RenderTimeMs() >= 0 ? absl::make_optional(Timestamp::Millis(RenderTimeMs())) diff --git a/api/video/encoded_frame.h b/api/video/encoded_frame.h index 610c988ebd..66aee227bb 100644 --- a/api/video/encoded_frame.h +++ b/api/video/encoded_frame.h @@ -34,6 +34,9 @@ class EncodedFrame : public webrtc::VCMEncodedFrame { // When this frame was received. // TODO(bugs.webrtc.org/13756): Use Timestamp instead of int. virtual int64_t ReceivedTime() const = 0; + // Returns a Timestamp from `ReceivedTime`, or nullopt if there is no receive + // time. + absl::optional ReceivedTimestamp() const; // When this frame should be rendered. // TODO(bugs.webrtc.org/13756): Use Timestamp instead of int. diff --git a/modules/video_coding/BUILD.gn b/modules/video_coding/BUILD.gn index a1b2d07067..7d6b5da1af 100644 --- a/modules/video_coding/BUILD.gn +++ b/modules/video_coding/BUILD.gn @@ -234,6 +234,20 @@ rtc_library("jitter_estimator") { absl_deps = [ "//third_party/abseil-cpp/absl/types:optional" ] } +rtc_library("inter_frame_delay") { + sources = [ + "inter_frame_delay.cc", + "inter_frame_delay.h", + ] + deps = [ + "..:module_api_public", + "../../api/units:frequency", + "../../api/units:time_delta", + "../../api/units:timestamp", + ] + absl_deps = [ "//third_party/abseil-cpp/absl/types:optional" ] +} + rtc_library("video_coding") { visibility = [ "*" ] sources = [ @@ -253,8 +267,6 @@ rtc_library("video_coding") { "h264_sps_pps_tracker.cc", "h264_sps_pps_tracker.h", "include/video_codec_initializer.h", - "inter_frame_delay.cc", - "inter_frame_delay.h", "internal_defines.h", "loss_notification_controller.cc", "loss_notification_controller.h", @@ -286,8 +298,8 @@ rtc_library("video_coding") { ":encoded_frame", ":frame_buffer", ":frame_helpers", + ":inter_frame_delay", ":jitter_estimator", - ":packet_buffer", ":rtt_filter", ":timing", ":video_codec_interface", @@ -402,6 +414,7 @@ rtc_library("video_coding_legacy") { deps = [ ":codec_globals_headers", ":encoded_frame", + ":inter_frame_delay", ":jitter_estimator", ":timing", ":video_codec_interface", @@ -1107,6 +1120,7 @@ if (rtc_include_tests) { "h264_sprop_parameter_sets_unittest.cc", "h264_sps_pps_tracker_unittest.cc", "histogram_unittest.cc", + "inter_frame_delay_unittest.cc", "jitter_buffer_unittest.cc", "jitter_estimator_tests.cc", "loss_notification_controller_unittest.cc", @@ -1151,6 +1165,7 @@ if (rtc_include_tests) { ":frame_buffer", ":frame_dependencies_calculator", ":h264_packet_buffer", + ":inter_frame_delay", ":jitter_estimator", ":nack_requester", ":packet_buffer", diff --git a/modules/video_coding/frame_buffer2.cc b/modules/video_coding/frame_buffer2.cc index d7944dbfa7..de239817e6 100644 --- a/modules/video_coding/frame_buffer2.cc +++ b/modules/video_coding/frame_buffer2.cc @@ -64,7 +64,6 @@ FrameBuffer::FrameBuffer(Clock* clock, callback_queue_(nullptr), jitter_estimator_(clock), timing_(timing), - inter_frame_delay_(clock_->TimeInMilliseconds()), stopped_(false), protection_mode_(kProtectionNack), stats_callback_(stats_callback), @@ -295,12 +294,11 @@ std::unique_ptr FrameBuffer::GetNextFrame() { } if (!superframe_delayed_by_retransmission) { - int64_t frame_delay; + auto frame_delay = inter_frame_delay_.CalculateDelay( + first_frame.Timestamp(), Timestamp::Millis(receive_time_ms)); - if (inter_frame_delay_.CalculateDelay(first_frame.Timestamp(), &frame_delay, - receive_time_ms)) { - jitter_estimator_.UpdateEstimate(TimeDelta::Millis(frame_delay), - superframe_size); + if (frame_delay) { + jitter_estimator_.UpdateEstimate(*frame_delay, superframe_size); } float rtt_mult = protection_mode_ == kProtectionNackFEC ? 0.0 : 1.0; diff --git a/modules/video_coding/inter_frame_delay.cc b/modules/video_coding/inter_frame_delay.cc index d0c21aa771..8cdb6644ae 100644 --- a/modules/video_coding/inter_frame_delay.cc +++ b/modules/video_coding/inter_frame_delay.cc @@ -10,85 +10,62 @@ #include "modules/video_coding/inter_frame_delay.h" +#include "absl/types/optional.h" +#include "api/units/frequency.h" +#include "api/units/time_delta.h" +#include "modules/include/module_common_types_public.h" + namespace webrtc { -VCMInterFrameDelay::VCMInterFrameDelay(int64_t currentWallClock) { - Reset(currentWallClock); +namespace { +constexpr Frequency k90kHz = Frequency::KiloHertz(90); +} + +VCMInterFrameDelay::VCMInterFrameDelay() { + Reset(); } // Resets the delay estimate. -void VCMInterFrameDelay::Reset(int64_t currentWallClock) { - _zeroWallClock = currentWallClock; - _wrapArounds = 0; - _prevWallClock = 0; - _prevTimestamp = 0; - _dTS = 0; +void VCMInterFrameDelay::Reset() { + prev_wall_clock_ = absl::nullopt; + prev_rtp_timestamp_unwrapped_ = 0; } // Calculates the delay of a frame with the given timestamp. // This method is called when the frame is complete. -bool VCMInterFrameDelay::CalculateDelay(uint32_t timestamp, - int64_t* delay, - int64_t currentWallClock) { - if (_prevWallClock == 0) { +absl::optional VCMInterFrameDelay::CalculateDelay( + uint32_t rtp_timestamp, + Timestamp now) { + int64_t rtp_timestamp_unwrapped = unwrapper_.Unwrap(rtp_timestamp); + if (!prev_wall_clock_) { // First set of data, initialization, wait for next frame. - _prevWallClock = currentWallClock; - _prevTimestamp = timestamp; - *delay = 0; - return true; + prev_wall_clock_ = now; + prev_rtp_timestamp_unwrapped_ = rtp_timestamp_unwrapped; + return TimeDelta::Zero(); } - int32_t prevWrapArounds = _wrapArounds; - CheckForWrapArounds(timestamp); - - // This will be -1 for backward wrap arounds and +1 for forward wrap arounds. - int32_t wrapAroundsSincePrev = _wrapArounds - prevWrapArounds; - // Account for reordering in jitter variance estimate in the future? // Note that this also captures incomplete frames which are grabbed for // decoding after a later frame has been complete, i.e. real packet losses. - if ((wrapAroundsSincePrev == 0 && timestamp < _prevTimestamp) || - wrapAroundsSincePrev < 0) { - *delay = 0; - return false; + uint32_t cropped_last = static_cast(prev_rtp_timestamp_unwrapped_); + if (rtp_timestamp_unwrapped < prev_rtp_timestamp_unwrapped_ || + !IsNewerTimestamp(rtp_timestamp, cropped_last)) { + return absl::nullopt; } - // Compute the compensated timestamp difference and convert it to ms and round - // it to closest integer. - _dTS = static_cast( - (timestamp + wrapAroundsSincePrev * (static_cast(1) << 32) - - _prevTimestamp) / - 90.0 + - 0.5); + // Compute the compensated timestamp difference. + int64_t d_rtp_ticks = rtp_timestamp_unwrapped - prev_rtp_timestamp_unwrapped_; + TimeDelta dts = d_rtp_ticks / k90kHz; + TimeDelta dt = now - *prev_wall_clock_; // frameDelay is the difference of dT and dTS -- i.e. the difference of the // wall clock time difference and the timestamp difference between two // following frames. - *delay = static_cast(currentWallClock - _prevWallClock - _dTS); + TimeDelta delay = dt - dts; - _prevTimestamp = timestamp; - _prevWallClock = currentWallClock; - - return true; + prev_rtp_timestamp_unwrapped_ = rtp_timestamp_unwrapped; + prev_wall_clock_ = now; + return delay; } -// Investigates if the timestamp clock has overflowed since the last timestamp -// and keeps track of the number of wrap arounds since reset. -void VCMInterFrameDelay::CheckForWrapArounds(uint32_t timestamp) { - if (timestamp < _prevTimestamp) { - // This difference will probably be less than -2^31 if we have had a wrap - // around (e.g. timestamp = 1, _prevTimestamp = 2^32 - 1). Since it is cast - // to a int32_t, it should be positive. - if (static_cast(timestamp - _prevTimestamp) > 0) { - // Forward wrap around. - _wrapArounds++; - } - // This difference will probably be less than -2^31 if we have had a - // backward wrap around. Since it is cast to a int32_t, it should be - // positive. - } else if (static_cast(_prevTimestamp - timestamp) > 0) { - // Backward wrap around. - _wrapArounds--; - } -} } // namespace webrtc diff --git a/modules/video_coding/inter_frame_delay.h b/modules/video_coding/inter_frame_delay.h index f121c61498..e0fee6bfba 100644 --- a/modules/video_coding/inter_frame_delay.h +++ b/modules/video_coding/inter_frame_delay.h @@ -13,46 +13,32 @@ #include +#include "absl/types/optional.h" +#include "api/units/time_delta.h" +#include "api/units/timestamp.h" +#include "modules/include/module_common_types_public.h" + namespace webrtc { class VCMInterFrameDelay { public: - explicit VCMInterFrameDelay(int64_t currentWallClock); + VCMInterFrameDelay(); // Resets the estimate. Zeros are given as parameters. - void Reset(int64_t currentWallClock); + void Reset(); // Calculates the delay of a frame with the given timestamp. // This method is called when the frame is complete. - // - // Input: - // - timestamp : RTP timestamp of a received frame. - // - *delay : Pointer to memory where the result should be - // stored. - // - currentWallClock : The current time in milliseconds. - // Should be -1 for normal operation, only used - // for testing. - // Return value : true if OK, false when reordered timestamps. - bool CalculateDelay(uint32_t timestamp, - int64_t* delay, - int64_t currentWallClock); + absl::optional CalculateDelay(uint32_t rtp_timestamp, + Timestamp now); private: - // Controls if the RTP timestamp counter has had a wrap around between the - // current and the previously received frame. - // - // Input: - // - timestamp : RTP timestamp of the current frame. - void CheckForWrapArounds(uint32_t timestamp); + // The previous rtp timestamp passed to the delay estimate + int64_t prev_rtp_timestamp_unwrapped_; + TimestampUnwrapper unwrapper_; - int64_t _zeroWallClock; // Local timestamp of the first video packet received - int32_t _wrapArounds; // Number of wrapArounds detected - // The previous timestamp passed to the delay estimate - uint32_t _prevTimestamp; // The previous wall clock timestamp used by the delay estimate - int64_t _prevWallClock; - // Wrap-around compensated difference between incoming timestamps - int64_t _dTS; + absl::optional prev_wall_clock_; }; } // namespace webrtc diff --git a/modules/video_coding/inter_frame_delay_unittest.cc b/modules/video_coding/inter_frame_delay_unittest.cc new file mode 100644 index 0000000000..a338ba9d3b --- /dev/null +++ b/modules/video_coding/inter_frame_delay_unittest.cc @@ -0,0 +1,190 @@ +/* + * Copyright (c) 2022 The WebRTC project authors. All Rights Reserved. + * + * Use of this source code is governed by a BSD-style license + * that can be found in the LICENSE file in the root of the source + * tree. An additional intellectual property rights grant can be found + * in the file PATENTS. All contributing project authors may + * be found in the AUTHORS file in the root of the source tree. + */ + +#include "modules/video_coding/inter_frame_delay.h" + +#include + +#include "absl/types/optional.h" +#include "api/units/frequency.h" +#include "api/units/time_delta.h" +#include "api/units/timestamp.h" +#include "system_wrappers/include/clock.h" +#include "test/gmock.h" +#include "test/gtest.h" + +namespace webrtc { + +namespace { + +// Test is for frames at 30fps. At 30fps, RTP timestamps will increase by +// 90000 / 30 = 3000 ticks per frame. +constexpr Frequency k30Fps = Frequency::Hertz(30); +constexpr TimeDelta kFrameDelay = 1 / k30Fps; +constexpr uint32_t kRtpTicksPerFrame = Frequency::KiloHertz(90) / k30Fps; +constexpr Timestamp kStartTime = Timestamp::Millis(1337); + +} // namespace + +using ::testing::Eq; +using ::testing::Optional; + +TEST(InterFrameDelayTest, OldRtpTimestamp) { + VCMInterFrameDelay inter_frame_delay; + EXPECT_THAT(inter_frame_delay.CalculateDelay(180000, kStartTime), + Optional(TimeDelta::Zero())); + EXPECT_THAT(inter_frame_delay.CalculateDelay(90000, kStartTime), + Eq(absl::nullopt)); +} + +TEST(InterFrameDelayTest, NegativeWrapAroundIsSameAsOldRtpTimestamp) { + VCMInterFrameDelay inter_frame_delay; + uint32_t rtp = 1500; + EXPECT_THAT(inter_frame_delay.CalculateDelay(rtp, kStartTime), + Optional(TimeDelta::Zero())); + // RTP has wrapped around backwards. + rtp -= 3000; + EXPECT_THAT(inter_frame_delay.CalculateDelay(rtp, kStartTime), + Eq(absl::nullopt)); +} + +TEST(InterFrameDelayTest, CorrectDelayForFrames) { + VCMInterFrameDelay inter_frame_delay; + // Use a fake clock to simplify time keeping. + SimulatedClock clock(kStartTime); + + // First frame is always delay 0. + uint32_t rtp = 90000; + EXPECT_THAT(inter_frame_delay.CalculateDelay(rtp, clock.CurrentTime()), + Optional(TimeDelta::Zero())); + + // Perfectly timed frame has 0 delay. + clock.AdvanceTime(kFrameDelay); + rtp += kRtpTicksPerFrame; + EXPECT_THAT(inter_frame_delay.CalculateDelay(rtp, clock.CurrentTime()), + Optional(TimeDelta::Zero())); + + // Slightly early frame will have a negative delay. + clock.AdvanceTime(kFrameDelay - TimeDelta::Millis(3)); + rtp += kRtpTicksPerFrame; + EXPECT_THAT(inter_frame_delay.CalculateDelay(rtp, clock.CurrentTime()), + Optional(-TimeDelta::Millis(3))); + + // Slightly late frame will have positive delay. + clock.AdvanceTime(kFrameDelay + TimeDelta::Micros(5125)); + rtp += kRtpTicksPerFrame; + EXPECT_THAT(inter_frame_delay.CalculateDelay(rtp, clock.CurrentTime()), + Optional(TimeDelta::Micros(5125))); + + // Simulate faster frame RTP at the same clock delay. The frame arrives late, + // since the RTP timestamp is faster than the delay, and thus is positive. + clock.AdvanceTime(kFrameDelay); + rtp += kRtpTicksPerFrame / 2; + EXPECT_THAT(inter_frame_delay.CalculateDelay(rtp, clock.CurrentTime()), + Optional(kFrameDelay / 2.0)); + + // Simulate slower frame RTP at the same clock delay. The frame is early, + // since the RTP timestamp advanced more than the delay, and thus is negative. + clock.AdvanceTime(kFrameDelay); + rtp += 1.5 * kRtpTicksPerFrame; + EXPECT_THAT(inter_frame_delay.CalculateDelay(rtp, clock.CurrentTime()), + Optional(-kFrameDelay / 2.0)); +} + +TEST(InterFrameDelayTest, PositiveWrapAround) { + VCMInterFrameDelay inter_frame_delay; + // Use a fake clock to simplify time keeping. + SimulatedClock clock(kStartTime); + + // First frame is behind the max RTP by 1500. + uint32_t rtp = std::numeric_limits::max() - 1500; + EXPECT_THAT(inter_frame_delay.CalculateDelay(rtp, clock.CurrentTime()), + Optional(TimeDelta::Zero())); + + // Rtp wraps around, now 1499. + rtp += kRtpTicksPerFrame; + + // Frame delay should be as normal, in this case simulated as 1ms late. + clock.AdvanceTime(kFrameDelay + TimeDelta::Millis(1)); + EXPECT_THAT(inter_frame_delay.CalculateDelay(rtp, clock.CurrentTime()), + Optional(TimeDelta::Millis(1))); +} + +TEST(InterFrameDelayTest, MultipleWrapArounds) { + // Simulate a long pauses which cause wrap arounds multiple times. + constexpr Frequency k90Khz = Frequency::KiloHertz(90); + constexpr uint32_t kHalfRtp = std::numeric_limits::max() / 2; + constexpr TimeDelta kWrapAroundDelay = kHalfRtp / k90Khz; + + VCMInterFrameDelay inter_frame_delay; + // Use a fake clock to simplify time keeping. + SimulatedClock clock(kStartTime); + uint32_t rtp = 0; + EXPECT_THAT(inter_frame_delay.CalculateDelay(rtp, clock.CurrentTime()), + Optional(TimeDelta::Zero())); + + rtp += kHalfRtp; + clock.AdvanceTime(kWrapAroundDelay); + EXPECT_THAT(inter_frame_delay.CalculateDelay(rtp, clock.CurrentTime()), + Optional(TimeDelta::Zero())); + // 1st wrap around. + rtp += kHalfRtp + 1; + clock.AdvanceTime(kWrapAroundDelay + TimeDelta::Millis(1)); + EXPECT_THAT(inter_frame_delay.CalculateDelay(rtp, clock.CurrentTime()), + Optional(TimeDelta::Millis(1) - (1 / k90Khz))); + + rtp += kHalfRtp; + clock.AdvanceTime(kWrapAroundDelay); + EXPECT_THAT(inter_frame_delay.CalculateDelay(rtp, clock.CurrentTime()), + Optional(TimeDelta::Zero())); + // 2nd wrap arounds. + rtp += kHalfRtp + 1; + clock.AdvanceTime(kWrapAroundDelay - TimeDelta::Millis(1)); + EXPECT_THAT(inter_frame_delay.CalculateDelay(rtp, clock.CurrentTime()), + Optional(-TimeDelta::Millis(1) - (1 / k90Khz))); + + // Ensure short delay (large RTP delay) between wrap-arounds has correct + // jitter. + rtp += kHalfRtp; + clock.AdvanceTime(TimeDelta::Millis(10)); + EXPECT_THAT(inter_frame_delay.CalculateDelay(rtp, clock.CurrentTime()), + Optional(-(kWrapAroundDelay - TimeDelta::Millis(10)))); + // 3nd wrap arounds, this time with large RTP delay. + rtp += kHalfRtp + 1; + clock.AdvanceTime(TimeDelta::Millis(10)); + EXPECT_THAT( + inter_frame_delay.CalculateDelay(rtp, clock.CurrentTime()), + Optional(-(kWrapAroundDelay - TimeDelta::Millis(10) + (1 / k90Khz)))); +} + +TEST(InterFrameDelayTest, NegativeWrapAroundAfterPositiveWrapAround) { + VCMInterFrameDelay inter_frame_delay; + // Use a fake clock to simplify time keeping. + SimulatedClock clock(kStartTime); + uint32_t rtp = std::numeric_limits::max() - 1500; + EXPECT_THAT(inter_frame_delay.CalculateDelay(rtp, clock.CurrentTime()), + Optional(TimeDelta::Zero())); + + // Rtp wraps around, now 1499. + rtp += kRtpTicksPerFrame; + // Frame delay should be as normal, in this case simulated as 1ms late. + clock.AdvanceTime(kFrameDelay); + EXPECT_THAT(inter_frame_delay.CalculateDelay(rtp, clock.CurrentTime()), + Optional(TimeDelta::Zero())); + + // Wrap back. + rtp -= kRtpTicksPerFrame; + // Frame delay should be as normal, in this case simulated as 1ms late. + clock.AdvanceTime(kFrameDelay); + EXPECT_THAT(inter_frame_delay.CalculateDelay(rtp, clock.CurrentTime()), + Eq(absl::nullopt)); +} + +} // namespace webrtc diff --git a/modules/video_coding/jitter_buffer.cc b/modules/video_coding/jitter_buffer.cc index a49ee6968c..f51b6ec898 100644 --- a/modules/video_coding/jitter_buffer.cc +++ b/modules/video_coding/jitter_buffer.cc @@ -9,11 +9,11 @@ */ #include "modules/video_coding/jitter_buffer.h" - #include #include #include +#include "api/units/timestamp.h" #include "modules/video_coding/frame_buffer.h" #include "modules/video_coding/include/video_coding.h" #include "modules/video_coding/inter_frame_delay.h" @@ -123,7 +123,6 @@ VCMJitterBuffer::VCMJitterBuffer(Clock* clock, num_packets_(0), num_duplicated_packets_(0), jitter_estimate_(clock), - inter_frame_delay_(clock_->TimeInMilliseconds()), missing_sequence_numbers_(SequenceNumberLessThan()), latest_received_sequence_number_(0), max_nack_list_size_(0), @@ -192,7 +191,7 @@ void VCMJitterBuffer::Flush() { num_consecutive_old_packets_ = 0; // Also reset the jitter and delay estimates jitter_estimate_.Reset(); - inter_frame_delay_.Reset(clock_->TimeInMilliseconds()); + inter_frame_delay_.Reset(); waiting_for_completion_.frame_size = 0; waiting_for_completion_.timestamp = 0; waiting_for_completion_.latest_packet_time = -1; @@ -392,13 +391,13 @@ VCMFrameBufferEnum VCMJitterBuffer::InsertPacket(const VCMPacket& packet, if (error != kNoError) return error; - int64_t now_ms = clock_->TimeInMilliseconds(); + Timestamp now = clock_->CurrentTime(); // We are keeping track of the first and latest seq numbers, and // the number of wraps to be able to calculate how many packets we expect. if (first_packet_since_reset_) { // Now it's time to start estimating jitter // reset the delay estimate. - inter_frame_delay_.Reset(now_ms); + inter_frame_delay_.Reset(); } // Empty packets may bias the jitter estimate (lacking size component), @@ -408,9 +407,9 @@ VCMFrameBufferEnum VCMJitterBuffer::InsertPacket(const VCMPacket& packet, // This can get bad if we have a lot of duplicate packets, // we will then count some packet multiple times. waiting_for_completion_.frame_size += packet.sizeBytes; - waiting_for_completion_.latest_packet_time = now_ms; + waiting_for_completion_.latest_packet_time = now.ms(); } else if (waiting_for_completion_.latest_packet_time >= 0 && - waiting_for_completion_.latest_packet_time + 2000 <= now_ms) { + waiting_for_completion_.latest_packet_time + 2000 <= now.ms()) { // A packet should never be more than two seconds late UpdateJitterEstimate(waiting_for_completion_, true); waiting_for_completion_.latest_packet_time = -1; @@ -425,7 +424,7 @@ VCMFrameBufferEnum VCMJitterBuffer::InsertPacket(const VCMPacket& packet, frame_data.rtt_ms = kDefaultRtt; frame_data.rolling_average_packets_per_frame = average_packets_per_frame_; VCMFrameBufferEnum buffer_state = - frame->InsertPacket(packet, now_ms, frame_data); + frame->InsertPacket(packet, now.ms(), frame_data); if (buffer_state > 0) { if (first_packet_since_reset_) { @@ -873,14 +872,14 @@ void VCMJitterBuffer::UpdateJitterEstimate(int64_t latest_packet_time_ms, if (latest_packet_time_ms == -1) { return; } - int64_t frame_delay; - bool not_reordered = inter_frame_delay_.CalculateDelay( - timestamp, &frame_delay, latest_packet_time_ms); + auto frame_delay = inter_frame_delay_.CalculateDelay( + timestamp, Timestamp::Millis(latest_packet_time_ms)); + + bool not_reordered = frame_delay.has_value(); // Filter out frames which have been reordered in time by the network if (not_reordered) { // Update the jitter estimate with the new samples - jitter_estimate_.UpdateEstimate(TimeDelta::Millis(frame_delay), - DataSize::Bytes(frame_size), + jitter_estimate_.UpdateEstimate(*frame_delay, DataSize::Bytes(frame_size), incomplete_frame); } } diff --git a/video/BUILD.gn b/video/BUILD.gn index e5ba48b3b2..8dcef1ae68 100644 --- a/video/BUILD.gn +++ b/video/BUILD.gn @@ -100,6 +100,7 @@ rtc_library("video") { "../modules/video_coding:codec_globals_headers", "../modules/video_coding:frame_buffer", "../modules/video_coding:frame_helpers", + "../modules/video_coding:inter_frame_delay", "../modules/video_coding:jitter_estimator", "../modules/video_coding:nack_requester", "../modules/video_coding:packet_buffer", diff --git a/video/frame_buffer_proxy.cc b/video/frame_buffer_proxy.cc index c7fce7ae19..2ec3ac41c9 100644 --- a/video/frame_buffer_proxy.cc +++ b/video/frame_buffer_proxy.cc @@ -149,7 +149,7 @@ struct FrameMetadata { contentType(frame.contentType()), delayed_by_retransmission(frame.delayed_by_retransmission()), rtp_timestamp(frame.Timestamp()), - receive_time_ms(frame.ReceivedTime()) {} + receive_time(frame.ReceivedTimestamp()) {} const bool is_last_spatial_layer; const bool is_keyframe; @@ -157,9 +157,15 @@ struct FrameMetadata { const VideoContentType contentType; const bool delayed_by_retransmission; const uint32_t rtp_timestamp; - const int64_t receive_time_ms; + const absl::optional receive_time; }; +Timestamp ReceiveTime(const EncodedFrame& frame) { + absl::optional ts = frame.ReceivedTimestamp(); + RTC_DCHECK(ts.has_value()) << "Received frame must have a timestamp set!"; + return *ts; +} + // Encapsulates use of the new frame buffer for use in VideoReceiveStream. This // behaves the same as the FrameBuffer2Proxy but uses frame_buffer3 instead. // Responsibilities from frame_buffer2, like stats, jitter and frame timing @@ -186,7 +192,6 @@ class FrameBuffer3Proxy : public FrameBufferProxy { timing_(timing), frame_decode_scheduler_(std::move(frame_decode_scheduler)), jitter_estimator_(clock_), - inter_frame_delay_(clock_->TimeInMilliseconds()), buffer_(std::make_unique(kMaxFramesBuffered, kMaxFramesHistory)), decode_timing_(clock_, timing_), @@ -248,12 +253,10 @@ class FrameBuffer3Proxy : public FrameBufferProxy { if (complete_units < buffer_->GetTotalNumberOfContinuousTemporalUnits()) { stats_proxy_->OnCompleteFrame(metadata.is_keyframe, metadata.size, metadata.contentType); - RTC_DCHECK_GE(metadata.receive_time_ms, 0) - << "Frame receive time must be positive for received frames, was " - << metadata.receive_time_ms << "."; - if (!metadata.delayed_by_retransmission && metadata.receive_time_ms >= 0) + RTC_DCHECK(metadata.receive_time) << "Frame receive time must be set!"; + if (!metadata.delayed_by_retransmission && metadata.receive_time) timing_->IncomingTimestamp(metadata.rtp_timestamp, - Timestamp::Millis(metadata.receive_time_ms)); + *metadata.receive_time); MaybeScheduleFrameForRelease(); } @@ -301,7 +304,7 @@ class FrameBuffer3Proxy : public FrameBufferProxy { bool superframe_delayed_by_retransmission = false; DataSize superframe_size = DataSize::Zero(); const EncodedFrame& first_frame = *frames.front(); - int64_t receive_time_ms = first_frame.ReceivedTime(); + Timestamp receive_time = ReceiveTime(first_frame); if (first_frame.is_keyframe()) keyframe_required_ = false; @@ -319,17 +322,15 @@ class FrameBuffer3Proxy : public FrameBufferProxy { superframe_delayed_by_retransmission |= frame->delayed_by_retransmission(); - receive_time_ms = std::max(receive_time_ms, frame->ReceivedTime()); + receive_time = std::max(receive_time, ReceiveTime(*frame)); superframe_size += DataSize::Bytes(frame->size()); } if (!superframe_delayed_by_retransmission) { - int64_t frame_delay; - - if (inter_frame_delay_.CalculateDelay(first_frame.Timestamp(), - &frame_delay, receive_time_ms)) { - jitter_estimator_.UpdateEstimate(TimeDelta::Millis(frame_delay), - superframe_size); + auto frame_delay = inter_frame_delay_.CalculateDelay( + first_frame.Timestamp(), receive_time); + if (frame_delay) { + jitter_estimator_.UpdateEstimate(*frame_delay, superframe_size); } float rtt_mult = protection_mode_ == kProtectionNackFEC ? 0.0 : 1.0;