From 558cabf67029f05878b25c9e63ec653ed3a1ad74 Mon Sep 17 00:00:00 2001 From: Ilya Nikolaevskiy Date: Tue, 14 Nov 2017 10:32:15 +0100 Subject: [PATCH] Refactor RtpToNtpEstimator and MovingMedianFilter MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In preparation for a new remote clock estimation: RtpToNtpEstimator will now store unwrapped rtp timestamps in |measurements| and |calculated| flag is out of |Parameters| struct. Additional missing |Reset| method added to MovingMedianFilter and PercentileFilter Bug: webrtc:8468 Change-Id: I1bb454ba0c93720cbb30bfded19677daaf9f721c Reviewed-on: https://webrtc-review.googlesource.com/22020 Commit-Queue: Ilya Nikolaevskiy Reviewed-by: Åsa Persson Reviewed-by: Erik Språng Reviewed-by: Karl Wiberg Cr-Commit-Position: refs/heads/master@{#20669} --- modules/BUILD.gn | 11 ++ modules/include/module_common_types.h | 89 +------------- modules/include/module_common_types_public.h | 110 ++++++++++++++++++ rtc_base/numerics/moving_median_filter.h | 10 ++ rtc_base/numerics/percentile_filter.h | 9 ++ system_wrappers/BUILD.gn | 2 + .../include/rtp_to_ntp_estimator.h | 30 ++--- .../source/rtp_to_ntp_estimator.cc | 92 +++++---------- .../source/rtp_to_ntp_estimator_unittest.cc | 57 ++++----- video/rtp_streams_synchronizer.cc | 2 +- 10 files changed, 218 insertions(+), 194 deletions(-) create mode 100644 modules/include/module_common_types_public.h diff --git a/modules/BUILD.gn b/modules/BUILD.gn index d534f569be..f0eb417bac 100644 --- a/modules/BUILD.gn +++ b/modules/BUILD.gn @@ -28,12 +28,23 @@ group("modules") { ] } +rtc_source_set("module_api_public") { + sources = [ + "include/module_common_types_public.h", + ] + deps = [ + "..:webrtc_common", + "../api:optional", + ] +} + rtc_source_set("module_api") { sources = [ "include/module.h", "include/module_common_types.h", ] deps = [ + ":module_api_public", "..:webrtc_common", "../api:optional", "../api:video_frame_api", diff --git a/modules/include/module_common_types.h b/modules/include/module_common_types.h index c3ad993555..c4594f5067 100644 --- a/modules/include/module_common_types.h +++ b/modules/include/module_common_types.h @@ -20,6 +20,7 @@ #include "api/optional.h" #include "api/video/video_rotation.h" #include "common_types.h" // NOLINT(build/include) +#include "modules/include/module_common_types_public.h" #include "modules/video_coding/codecs/h264/include/h264_globals.h" #include "modules/video_coding/codecs/vp8/include/vp8_globals.h" #include "modules/video_coding/codecs/vp9/include/vp9_globals.h" @@ -568,94 +569,6 @@ inline AudioFrame& AudioFrame::operator+=(const AudioFrame& rhs) { return *this; } -template -inline bool IsNewer(U value, U prev_value) { - static_assert(!std::numeric_limits::is_signed, "U must be unsigned"); - // kBreakpoint is the half-way mark for the type U. For instance, for a - // uint16_t it will be 0x8000, and for a uint32_t, it will be 0x8000000. - constexpr U kBreakpoint = (std::numeric_limits::max() >> 1) + 1; - // Distinguish between elements that are exactly kBreakpoint apart. - // If t1>t2 and |t1-t2| = kBreakpoint: IsNewer(t1,t2)=true, - // IsNewer(t2,t1)=false - // rather than having IsNewer(t1,t2) = IsNewer(t2,t1) = false. - if (value - prev_value == kBreakpoint) { - return value > prev_value; - } - return value != prev_value && - static_cast(value - prev_value) < kBreakpoint; -} - -inline bool IsNewerSequenceNumber(uint16_t sequence_number, - uint16_t prev_sequence_number) { - return IsNewer(sequence_number, prev_sequence_number); -} - -inline bool IsNewerTimestamp(uint32_t timestamp, uint32_t prev_timestamp) { - return IsNewer(timestamp, prev_timestamp); -} - -inline uint16_t LatestSequenceNumber(uint16_t sequence_number1, - uint16_t sequence_number2) { - return IsNewerSequenceNumber(sequence_number1, sequence_number2) - ? sequence_number1 - : sequence_number2; -} - -inline uint32_t LatestTimestamp(uint32_t timestamp1, uint32_t timestamp2) { - return IsNewerTimestamp(timestamp1, timestamp2) ? timestamp1 : timestamp2; -} - -// Utility class to unwrap a number to a larger type. The numbers will never be -// unwrapped to a negative value. -template -class Unwrapper { - static_assert(!std::numeric_limits::is_signed, "U must be unsigned"); - static_assert(std::numeric_limits::max() <= - std::numeric_limits::max(), - "U must not be wider than 32 bits"); - - public: - // Get the unwrapped value, but don't update the internal state. - int64_t UnwrapWithoutUpdate(U value) { - if (!last_value_) - return value; - - constexpr int64_t kMaxPlusOne = - static_cast(std::numeric_limits::max()) + 1; - - U cropped_last = static_cast(*last_value_); - int64_t delta = value - cropped_last; - if (IsNewer(value, cropped_last)) { - if (delta < 0) - delta += kMaxPlusOne; // Wrap forwards. - } else if (delta > 0 && (*last_value_ + delta - kMaxPlusOne) >= 0) { - // If value is older but delta is positive, this is a backwards - // wrap-around. However, don't wrap backwards past 0 (unwrapped). - delta -= kMaxPlusOne; - } - - return *last_value_ + delta; - } - - // Only update the internal state to the specified last (unwrapped) value. - void UpdateLast(int64_t last_value) { - last_value_ = rtc::Optional(last_value); - } - - // Unwrap the value and update the internal state. - int64_t Unwrap(U value) { - int64_t unwrapped = UnwrapWithoutUpdate(value); - UpdateLast(unwrapped); - return unwrapped; - } - - private: - rtc::Optional last_value_; -}; - -using SequenceNumberUnwrapper = Unwrapper; -using TimestampUnwrapper = Unwrapper; - struct PacedPacketInfo { PacedPacketInfo() {} PacedPacketInfo(int probe_cluster_id, diff --git a/modules/include/module_common_types_public.h b/modules/include/module_common_types_public.h new file mode 100644 index 0000000000..d73fae2682 --- /dev/null +++ b/modules/include/module_common_types_public.h @@ -0,0 +1,110 @@ +/* + * Copyright (c) 2017 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. + */ + +#ifndef MODULES_INCLUDE_MODULE_COMMON_TYPES_PUBLIC_H_ +#define MODULES_INCLUDE_MODULE_COMMON_TYPES_PUBLIC_H_ + +#include + +#include "api/optional.h" +#include "typedefs.h" // NOLINT(build/include) + +namespace webrtc { + +template +inline bool IsNewer(U value, U prev_value) { + static_assert(!std::numeric_limits::is_signed, "U must be unsigned"); + // kBreakpoint is the half-way mark for the type U. For instance, for a + // uint16_t it will be 0x8000, and for a uint32_t, it will be 0x8000000. + constexpr U kBreakpoint = (std::numeric_limits::max() >> 1) + 1; + // Distinguish between elements that are exactly kBreakpoint apart. + // If t1>t2 and |t1-t2| = kBreakpoint: IsNewer(t1,t2)=true, + // IsNewer(t2,t1)=false + // rather than having IsNewer(t1,t2) = IsNewer(t2,t1) = false. + if (value - prev_value == kBreakpoint) { + return value > prev_value; + } + return value != prev_value && + static_cast(value - prev_value) < kBreakpoint; +} + +// Utility class to unwrap a number to a larger type. The numbers will never be +// unwrapped to a negative value. +template +class Unwrapper { + static_assert(!std::numeric_limits::is_signed, "U must be unsigned"); + static_assert(std::numeric_limits::max() <= + std::numeric_limits::max(), + "U must not be wider than 32 bits"); + + public: + // Get the unwrapped value, but don't update the internal state. + int64_t UnwrapWithoutUpdate(U value) { + if (!last_value_) + return value; + + constexpr int64_t kMaxPlusOne = + static_cast(std::numeric_limits::max()) + 1; + + U cropped_last = static_cast(*last_value_); + int64_t delta = value - cropped_last; + if (IsNewer(value, cropped_last)) { + if (delta < 0) + delta += kMaxPlusOne; // Wrap forwards. + } else if (delta > 0 && (*last_value_ + delta - kMaxPlusOne) >= 0) { + // If value is older but delta is positive, this is a backwards + // wrap-around. However, don't wrap backwards past 0 (unwrapped). + delta -= kMaxPlusOne; + } + + return *last_value_ + delta; + } + + // Only update the internal state to the specified last (unwrapped) value. + void UpdateLast(int64_t last_value) { + last_value_ = rtc::Optional(last_value); + } + + // Unwrap the value and update the internal state. + int64_t Unwrap(U value) { + int64_t unwrapped = UnwrapWithoutUpdate(value); + UpdateLast(unwrapped); + return unwrapped; + } + + private: + rtc::Optional last_value_; +}; + +using SequenceNumberUnwrapper = Unwrapper; +using TimestampUnwrapper = Unwrapper; + +inline bool IsNewerSequenceNumber(uint16_t sequence_number, + uint16_t prev_sequence_number) { + return IsNewer(sequence_number, prev_sequence_number); +} + +inline bool IsNewerTimestamp(uint32_t timestamp, uint32_t prev_timestamp) { + return IsNewer(timestamp, prev_timestamp); +} + +inline uint16_t LatestSequenceNumber(uint16_t sequence_number1, + uint16_t sequence_number2) { + return IsNewerSequenceNumber(sequence_number1, sequence_number2) + ? sequence_number1 + : sequence_number2; +} + +inline uint32_t LatestTimestamp(uint32_t timestamp1, uint32_t timestamp2) { + return IsNewerTimestamp(timestamp1, timestamp2) ? timestamp1 : timestamp2; +} + +} // namespace webrtc +#endif // MODULES_INCLUDE_MODULE_COMMON_TYPES_PUBLIC_H_ diff --git a/rtc_base/numerics/moving_median_filter.h b/rtc_base/numerics/moving_median_filter.h index 4b42d4a213..b5c5fcea9e 100644 --- a/rtc_base/numerics/moving_median_filter.h +++ b/rtc_base/numerics/moving_median_filter.h @@ -30,6 +30,9 @@ class MovingMedianFilter { // Insert a new sample. void Insert(const T& value); + // Removes all samples; + void Reset(); + // Get median over the latest window. T GetFilteredValue() const; @@ -65,5 +68,12 @@ T MovingMedianFilter::GetFilteredValue() const { return percentile_filter_.GetPercentileValue(); } +template +void MovingMedianFilter::Reset() { + percentile_filter_.Reset(); + samples_.clear(); + samples_stored_ = 0; +} + } // namespace webrtc #endif // RTC_BASE_NUMERICS_MOVING_MEDIAN_FILTER_H_ diff --git a/rtc_base/numerics/percentile_filter.h b/rtc_base/numerics/percentile_filter.h index c24850bed8..cba44639b7 100644 --- a/rtc_base/numerics/percentile_filter.h +++ b/rtc_base/numerics/percentile_filter.h @@ -41,6 +41,9 @@ class PercentileFilter { // Get the percentile value. The complexity of this operation is constant. T GetPercentileValue() const; + // Removes all the stored observations. + void Reset(); + private: // Update iterator and index to point at target percentile value. void UpdatePercentileIterator(); @@ -110,6 +113,12 @@ T PercentileFilter::GetPercentileValue() const { return set_.empty() ? 0 : *percentile_it_; } +template +void PercentileFilter::Reset() { + set_.clear(); + percentile_it_ = set_.begin(); + percentile_index_ = 0; +} } // namespace webrtc #endif // RTC_BASE_NUMERICS_PERCENTILE_FILTER_H_ diff --git a/system_wrappers/BUILD.gn b/system_wrappers/BUILD.gn index 6183ee5ff0..fb376fe3f6 100644 --- a/system_wrappers/BUILD.gn +++ b/system_wrappers/BUILD.gn @@ -51,6 +51,8 @@ rtc_static_library("system_wrappers") { libs = [] deps = [ "..:webrtc_common", + "../api:optional", + "../modules:module_api_public", ] public_deps = [ ":cpu_features_api", diff --git a/system_wrappers/include/rtp_to_ntp_estimator.h b/system_wrappers/include/rtp_to_ntp_estimator.h index f84b61d2f8..6aad545b16 100644 --- a/system_wrappers/include/rtp_to_ntp_estimator.h +++ b/system_wrappers/include/rtp_to_ntp_estimator.h @@ -13,6 +13,8 @@ #include +#include "api/optional.h" +#include "modules/include/module_common_types_public.h" #include "system_wrappers/include/ntp_time.h" #include "typedefs.h" // NOLINT(build/include) @@ -27,18 +29,19 @@ class RtpToNtpEstimator { // RTP and NTP timestamp pair from a RTCP SR report. struct RtcpMeasurement { - RtcpMeasurement(uint32_t ntp_secs, uint32_t ntp_frac, uint32_t timestamp); + RtcpMeasurement(uint32_t ntp_secs, + uint32_t ntp_frac, + int64_t unwrapped_timestamp); bool IsEqual(const RtcpMeasurement& other) const; NtpTime ntp_time; - uint32_t rtp_timestamp; + int64_t unwrapped_rtp_timestamp; }; // Estimated parameters from RTP and NTP timestamp pairs in |measurements_|. struct Parameters { - double frequency_khz = 0.0; - double offset_ms = 0.0; - bool calculated = false; + double frequency_khz; + double offset_ms; }; // Updates measurements with RTP/NTP timestamp pair from a RTCP sender report. @@ -52,7 +55,13 @@ class RtpToNtpEstimator { // Returns true on success, false otherwise. bool Estimate(int64_t rtp_timestamp, int64_t* rtp_timestamp_ms) const; - const Parameters& params() const { return params_; } + const rtc::Optional params() const { + rtc::Optional res; + if (params_calculated_) { + res.emplace(params_); + } + return res; + } static const int kMaxInvalidSamples = 3; @@ -62,14 +71,9 @@ class RtpToNtpEstimator { int consecutive_invalid_samples_; std::list measurements_; Parameters params_; + bool params_calculated_; + mutable TimestampUnwrapper unwrapper_; }; - -// Returns: -// 1: forward wrap around. -// 0: no wrap around. -// -1: backwards wrap around (i.e. reordering). -int CheckForWrapArounds(uint32_t new_timestamp, uint32_t old_timestamp); - } // namespace webrtc #endif // SYSTEM_WRAPPERS_INCLUDE_RTP_TO_NTP_ESTIMATOR_H_ diff --git a/system_wrappers/source/rtp_to_ntp_estimator.cc b/system_wrappers/source/rtp_to_ntp_estimator.cc index 3728481933..cd33f32020 100644 --- a/system_wrappers/source/rtp_to_ntp_estimator.cc +++ b/system_wrappers/source/rtp_to_ntp_estimator.cc @@ -33,20 +33,6 @@ bool CalculateFrequency(int64_t ntp_ms1, return true; } -// Detects if there has been a wraparound between |old_timestamp| and -// |new_timestamp|, and compensates by adding 2^32 if that is the case. -bool CompensateForWrapAround(uint32_t new_timestamp, - uint32_t old_timestamp, - int64_t* compensated_timestamp) { - int64_t wraps = CheckForWrapArounds(new_timestamp, old_timestamp); - if (wraps < 0) { - // Reordering, don't use this packet. - return false; - } - *compensated_timestamp = new_timestamp + (wraps << 32); - return true; -} - bool Contains(const std::list& measurements, const RtpToNtpEstimator::RtcpMeasurement& other) { for (const auto& measurement : measurements) { @@ -59,28 +45,29 @@ bool Contains(const std::list& measurements, RtpToNtpEstimator::RtcpMeasurement::RtcpMeasurement(uint32_t ntp_secs, uint32_t ntp_frac, - uint32_t timestamp) - : ntp_time(ntp_secs, ntp_frac), rtp_timestamp(timestamp) {} + int64_t unwrapped_timestamp) + : ntp_time(ntp_secs, ntp_frac), + unwrapped_rtp_timestamp(unwrapped_timestamp) {} bool RtpToNtpEstimator::RtcpMeasurement::IsEqual( const RtcpMeasurement& other) const { // Use || since two equal timestamps will result in zero frequency and in // RtpToNtpMs, |rtp_timestamp_ms| is estimated by dividing by the frequency. - return (ntp_time == other.ntp_time) || (rtp_timestamp == other.rtp_timestamp); + return (ntp_time == other.ntp_time) || + (unwrapped_rtp_timestamp == other.unwrapped_rtp_timestamp); } // Class for converting an RTP timestamp to the NTP domain. -RtpToNtpEstimator::RtpToNtpEstimator() : consecutive_invalid_samples_(0) {} +RtpToNtpEstimator::RtpToNtpEstimator() + : consecutive_invalid_samples_(0), params_calculated_(false) {} RtpToNtpEstimator::~RtpToNtpEstimator() {} void RtpToNtpEstimator::UpdateParameters() { if (measurements_.size() != kNumRtcpReportsToUse) return; - int64_t timestamp_new = measurements_.front().rtp_timestamp; - int64_t timestamp_old = measurements_.back().rtp_timestamp; - if (!CompensateForWrapAround(timestamp_new, timestamp_old, ×tamp_new)) - return; + int64_t timestamp_new = measurements_.front().unwrapped_rtp_timestamp; + int64_t timestamp_old = measurements_.back().unwrapped_rtp_timestamp; int64_t ntp_ms_new = measurements_.front().ntp_time.ToMs(); int64_t ntp_ms_old = measurements_.back().ntp_time.ToMs(); @@ -90,7 +77,7 @@ void RtpToNtpEstimator::UpdateParameters() { return; } params_.offset_ms = timestamp_new - params_.frequency_khz * ntp_ms_new; - params_.calculated = true; + params_calculated_ = true; } bool RtpToNtpEstimator::UpdateMeasurements(uint32_t ntp_secs, @@ -99,7 +86,10 @@ bool RtpToNtpEstimator::UpdateMeasurements(uint32_t ntp_secs, bool* new_rtcp_sr) { *new_rtcp_sr = false; - RtcpMeasurement new_measurement(ntp_secs, ntp_frac, rtp_timestamp); + int64_t unwrapped_rtp_timestamp = unwrapper_.Unwrap(rtp_timestamp); + + RtcpMeasurement new_measurement(ntp_secs, ntp_frac, unwrapped_rtp_timestamp); + if (Contains(measurements_, new_measurement)) { // RTCP SR report already added. return true; @@ -109,25 +99,21 @@ bool RtpToNtpEstimator::UpdateMeasurements(uint32_t ntp_secs, int64_t ntp_ms_new = new_measurement.ntp_time.ToMs(); bool invalid_sample = false; - for (const auto& measurement : measurements_) { - if (ntp_ms_new <= measurement.ntp_time.ToMs()) { - // Old report. + if (!measurements_.empty()) { + int64_t old_rtp_timestamp = measurements_.front().unwrapped_rtp_timestamp; + int64_t old_ntp_ms = measurements_.front().ntp_time.ToMs(); + if (ntp_ms_new <= old_ntp_ms) { invalid_sample = true; - break; - } - int64_t timestamp_new = new_measurement.rtp_timestamp; - if (!CompensateForWrapAround(timestamp_new, measurement.rtp_timestamp, - ×tamp_new)) { - invalid_sample = true; - break; - } - if (timestamp_new <= measurement.rtp_timestamp) { + } else if (unwrapped_rtp_timestamp <= old_rtp_timestamp) { RTC_LOG(LS_WARNING) << "Newer RTCP SR report with older RTP timestamp, dropping"; invalid_sample = true; - break; + } else if (unwrapped_rtp_timestamp - old_rtp_timestamp > (1 << 25)) { + // Sanity check. No jumps too far into the future in rtp. + invalid_sample = true; } } + if (invalid_sample) { ++consecutive_invalid_samples_; if (consecutive_invalid_samples_ < kMaxInvalidSamples) { @@ -136,6 +122,7 @@ bool RtpToNtpEstimator::UpdateMeasurements(uint32_t ntp_secs, RTC_LOG(LS_WARNING) << "Multiple consecutively invalid RTCP SR reports, " "clearing measurements."; measurements_.clear(); + params_calculated_ = false; } consecutive_invalid_samples_ = 0; @@ -153,18 +140,13 @@ bool RtpToNtpEstimator::UpdateMeasurements(uint32_t ntp_secs, bool RtpToNtpEstimator::Estimate(int64_t rtp_timestamp, int64_t* rtp_timestamp_ms) const { - if (!params_.calculated || measurements_.empty()) + if (!params_calculated_) return false; - uint32_t rtp_timestamp_old = measurements_.back().rtp_timestamp; - int64_t rtp_timestamp_unwrapped; - if (!CompensateForWrapAround(rtp_timestamp, rtp_timestamp_old, - &rtp_timestamp_unwrapped)) { - return false; - } + int64_t rtp_timestamp_unwrapped = unwrapper_.Unwrap(rtp_timestamp); - // params_.calculated should not be true unless params_.frequency_khz has been - // set to something non-zero. + // params_calculated_ should not be true unless ms params_.frequency_khz has + // been calculated to something non zero. RTC_DCHECK_NE(params_.frequency_khz, 0.0); double rtp_ms = (static_cast(rtp_timestamp_unwrapped) - params_.offset_ms) / @@ -177,22 +159,4 @@ bool RtpToNtpEstimator::Estimate(int64_t rtp_timestamp, *rtp_timestamp_ms = rtp_ms; return true; } - -int CheckForWrapArounds(uint32_t new_timestamp, uint32_t old_timestamp) { - if (new_timestamp < old_timestamp) { - // This difference should be less than -2^31 if we have had a wrap around - // (e.g. |new_timestamp| = 1, |rtcp_rtp_timestamp| = 2^32 - 1). Since it is - // cast to a int32_t, it should be positive. - if (static_cast(new_timestamp - old_timestamp) > 0) { - // Forward wrap around. - return 1; - } - } else if (static_cast(old_timestamp - new_timestamp) > 0) { - // This difference should 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. - return -1; - } - return 0; -} - } // namespace webrtc diff --git a/system_wrappers/source/rtp_to_ntp_estimator_unittest.cc b/system_wrappers/source/rtp_to_ntp_estimator_unittest.cc index 6bf12f40e8..0647ec8cad 100644 --- a/system_wrappers/source/rtp_to_ntp_estimator_unittest.cc +++ b/system_wrappers/source/rtp_to_ntp_estimator_unittest.cc @@ -17,26 +17,6 @@ const uint32_t kOneMsInNtpFrac = 4294967; const uint32_t kTimestampTicksPerMs = 90; } // namespace -TEST(WrapAroundTests, NoWrap) { - EXPECT_EQ(0, CheckForWrapArounds(0xFFFFFFFF, 0xFFFFFFFE)); - EXPECT_EQ(0, CheckForWrapArounds(1, 0)); - EXPECT_EQ(0, CheckForWrapArounds(0x00010000, 0x0000FFFF)); -} - -TEST(WrapAroundTests, ForwardWrap) { - EXPECT_EQ(1, CheckForWrapArounds(0, 0xFFFFFFFF)); - EXPECT_EQ(1, CheckForWrapArounds(0, 0xFFFF0000)); - EXPECT_EQ(1, CheckForWrapArounds(0x0000FFFF, 0xFFFFFFFF)); - EXPECT_EQ(1, CheckForWrapArounds(0x0000FFFF, 0xFFFF0000)); -} - -TEST(WrapAroundTests, BackwardWrap) { - EXPECT_EQ(-1, CheckForWrapArounds(0xFFFFFFFF, 0)); - EXPECT_EQ(-1, CheckForWrapArounds(0xFFFF0000, 0)); - EXPECT_EQ(-1, CheckForWrapArounds(0xFFFFFFFF, 0x0000FFFF)); - EXPECT_EQ(-1, CheckForWrapArounds(0xFFFF0000, 0x0000FFFF)); -} - TEST(WrapAroundTests, OldRtcpWrapped_OldRtpTimestamp) { RtpToNtpEstimator estimator; bool new_sr; @@ -47,8 +27,29 @@ TEST(WrapAroundTests, OldRtcpWrapped_OldRtpTimestamp) { estimator.UpdateMeasurements(ntp_sec, ntp_frac, timestamp, &new_sr)); ntp_frac += kOneMsInNtpFrac; timestamp -= kTimestampTicksPerMs; + // No wraparound will be detected, since we are not allowed to wrap below 0, + // but there will be huge rtp timestamp jump, e.g. old_timestamp = 0, + // new_timestamp = 4294967295, which should be detected. + EXPECT_FALSE( + estimator.UpdateMeasurements(ntp_sec, ntp_frac, timestamp, &new_sr)); +} + +TEST(WrapAroundTests, OldRtcpWrapped_OldRtpTimestamp_Wraparound_Detected) { + RtpToNtpEstimator estimator; + bool new_sr; + uint32_t ntp_sec = 0; + uint32_t ntp_frac = 1; + uint32_t timestamp = 0xFFFFFFFE; + EXPECT_TRUE( + estimator.UpdateMeasurements(ntp_sec, ntp_frac, timestamp, &new_sr)); + ntp_frac += 2 * kOneMsInNtpFrac; + timestamp += 2 * kTimestampTicksPerMs; + EXPECT_TRUE( + estimator.UpdateMeasurements(ntp_sec, ntp_frac, timestamp, &new_sr)); + ntp_frac += kOneMsInNtpFrac; + timestamp -= kTimestampTicksPerMs; // Expected to fail since the older RTCP has a smaller RTP timestamp than the - // newer (old:0, new:4294967206). + // newer (old:10, new:4294967206). EXPECT_FALSE( estimator.UpdateMeasurements(ntp_sec, ntp_frac, timestamp, &new_sr)); } @@ -106,7 +107,7 @@ TEST(WrapAroundTests, OldRtp_RtcpsWrapped) { bool new_sr; uint32_t ntp_sec = 0; uint32_t ntp_frac = 1; - uint32_t timestamp = 0; + uint32_t timestamp = 0xFFFFFFFF; EXPECT_TRUE( estimator.UpdateMeasurements(ntp_sec, ntp_frac, timestamp, &new_sr)); ntp_frac += kOneMsInNtpFrac; @@ -114,7 +115,7 @@ TEST(WrapAroundTests, OldRtp_RtcpsWrapped) { EXPECT_TRUE( estimator.UpdateMeasurements(ntp_sec, ntp_frac, timestamp, &new_sr)); timestamp -= 2 * kTimestampTicksPerMs; - int64_t timestamp_ms = -1; + int64_t timestamp_ms = 0xFFFFFFFF; EXPECT_FALSE(estimator.Estimate(timestamp, ×tamp_ms)); } @@ -265,15 +266,15 @@ TEST(UpdateRtcpMeasurementTests, VerifyParameters) { EXPECT_TRUE( estimator.UpdateMeasurements(ntp_sec, ntp_frac, timestamp, &new_sr)); EXPECT_TRUE(new_sr); - EXPECT_FALSE(estimator.params().calculated); + EXPECT_FALSE(estimator.params()); // Add second report, parameters should be calculated. ntp_frac += kOneMsInNtpFrac; timestamp += kTimestampTicksPerMs; EXPECT_TRUE( estimator.UpdateMeasurements(ntp_sec, ntp_frac, timestamp, &new_sr)); - EXPECT_TRUE(estimator.params().calculated); - EXPECT_DOUBLE_EQ(90.0, estimator.params().frequency_khz); - EXPECT_NE(0.0, estimator.params().offset_ms); + EXPECT_TRUE(estimator.params()); + EXPECT_DOUBLE_EQ(90.0, estimator.params()->frequency_khz); + EXPECT_NE(0.0, estimator.params()->offset_ms); } TEST(RtpToNtpTests, FailsForNoParameters) { @@ -286,7 +287,7 @@ TEST(RtpToNtpTests, FailsForNoParameters) { estimator.UpdateMeasurements(ntp_sec, ntp_frac, timestamp, &new_sr)); EXPECT_TRUE(new_sr); // Parameters are not calculated, conversion of RTP to NTP time should fail. - EXPECT_FALSE(estimator.params().calculated); + EXPECT_FALSE(estimator.params()); int64_t timestamp_ms = -1; EXPECT_FALSE(estimator.Estimate(timestamp, ×tamp_ms)); } diff --git a/video/rtp_streams_synchronizer.cc b/video/rtp_streams_synchronizer.cc index 0274849aeb..c330a4434a 100644 --- a/video/rtp_streams_synchronizer.cc +++ b/video/rtp_streams_synchronizer.cc @@ -147,7 +147,7 @@ bool RtpStreamsSynchronizer::GetStreamSyncOffsetInMs( latest_video_ntp += time_to_render_ms; *stream_offset_ms = latest_audio_ntp - latest_video_ntp; - *estimated_freq_khz = video_measurement_.rtp_to_ntp.params().frequency_khz; + *estimated_freq_khz = video_measurement_.rtp_to_ntp.params()->frequency_khz; return true; }