Revert "Use moving median filters in RemoteNtpTimeEstimator, RtpToNtpEstimator"
This reverts commit 550b666e20a13f9c22effce878a8e0078a0f7bad. Reason for revert: breaks downstream projects. Original change's description: > Use moving median filters in RemoteNtpTimeEstimator, RtpToNtpEstimator > > If Webrtc-ClockEstimation experiment is enabled, median filtering is > applied to results of RtpToNtpEstimator and RemoteNtpEstimator to smooth > out random errors introduced by incorrect RTCP SR reports and networking > delays. > > Bug: webrtc:8468 > Change-Id: Iec6d094d2809d1efeb0b9483059167d9a03880da > Reviewed-on: https://webrtc-review.googlesource.com/22682 > Commit-Queue: Ilya Nikolaevskiy <ilnik@webrtc.org> > Reviewed-by: Åsa Persson <asapersson@webrtc.org> > Reviewed-by: Per Kjellander <perkj@webrtc.org> > Cr-Commit-Position: refs/heads/master@{#20682} TBR=ilnik@webrtc.org,asapersson@webrtc.org,perkj@webrtc.org Change-Id: I17345d912bbaf635612c9b399d5f9166de818190 No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: webrtc:8468 Reviewed-on: https://webrtc-review.googlesource.com/23320 Reviewed-by: Mirko Bonadei <mbonadei@webrtc.org> Commit-Queue: Mirko Bonadei <mbonadei@webrtc.org> Cr-Commit-Position: refs/heads/master@{#20689}
This commit is contained in:

committed by
Commit Bot

parent
e403212ede
commit
f6703c4dcb
@ -101,10 +101,7 @@ rtc_static_library("system_wrappers") {
|
||||
suppressed_configs += [ "//build/config/clang:find_bad_constructs" ]
|
||||
}
|
||||
|
||||
deps += [
|
||||
"../rtc_base:rtc_base_approved",
|
||||
"../rtc_base:rtc_numerics",
|
||||
]
|
||||
deps += [ "../rtc_base:rtc_base_approved" ]
|
||||
}
|
||||
|
||||
rtc_source_set("cpu_features_api") {
|
||||
|
@ -15,7 +15,6 @@
|
||||
|
||||
#include "api/optional.h"
|
||||
#include "modules/include/module_common_types_public.h"
|
||||
#include "rtc_base/numerics/moving_median_filter.h"
|
||||
#include "system_wrappers/include/ntp_time.h"
|
||||
#include "typedefs.h" // NOLINT(build/include)
|
||||
|
||||
@ -41,23 +40,8 @@ class RtpToNtpEstimator {
|
||||
|
||||
// Estimated parameters from RTP and NTP timestamp pairs in |measurements_|.
|
||||
struct Parameters {
|
||||
// Implicit conversion from int because MovingMedianFilter returns 0
|
||||
// internally if no samples are present. However, it should never happen as
|
||||
// we don't ask smoothing_filter_ to return anything if there were no
|
||||
// samples.
|
||||
Parameters(const int& value) { // NOLINT
|
||||
RTC_NOTREACHED();
|
||||
}
|
||||
Parameters() : frequency_khz(0.0), offset_ms(0.0) {}
|
||||
|
||||
double frequency_khz;
|
||||
double offset_ms;
|
||||
|
||||
// Needed to make it work inside MovingMedianFilter
|
||||
bool operator<(const Parameters& other) const;
|
||||
bool operator==(const Parameters& other) const;
|
||||
bool operator<=(const Parameters& other) const;
|
||||
bool operator!=(const Parameters& other) const;
|
||||
};
|
||||
|
||||
// Updates measurements with RTP/NTP timestamp pair from a RTCP sender report.
|
||||
@ -71,8 +55,13 @@ class RtpToNtpEstimator {
|
||||
// Returns true on success, false otherwise.
|
||||
bool Estimate(int64_t rtp_timestamp, int64_t* rtp_timestamp_ms) const;
|
||||
|
||||
// Returns estimated rtp to ntp linear transform parameters.
|
||||
const rtc::Optional<Parameters> params() const;
|
||||
const rtc::Optional<Parameters> params() const {
|
||||
rtc::Optional<Parameters> res;
|
||||
if (params_calculated_) {
|
||||
res.emplace(params_);
|
||||
}
|
||||
return res;
|
||||
}
|
||||
|
||||
static const int kMaxInvalidSamples = 3;
|
||||
|
||||
@ -82,10 +71,8 @@ class RtpToNtpEstimator {
|
||||
int consecutive_invalid_samples_;
|
||||
std::list<RtcpMeasurement> measurements_;
|
||||
Parameters params_;
|
||||
MovingMedianFilter<Parameters> smoothing_filter_;
|
||||
bool params_calculated_;
|
||||
mutable TimestampUnwrapper unwrapper_;
|
||||
const bool is_experiment_enabled_;
|
||||
};
|
||||
} // namespace webrtc
|
||||
|
||||
|
@ -13,18 +13,11 @@
|
||||
#include "rtc_base/checks.h"
|
||||
#include "rtc_base/logging.h"
|
||||
#include "system_wrappers/include/clock.h"
|
||||
#include "system_wrappers/include/field_trial.h"
|
||||
|
||||
namespace webrtc {
|
||||
namespace {
|
||||
// Number of RTCP SR reports to use to map between RTP and NTP.
|
||||
const size_t kNumRtcpReportsToUse = 2;
|
||||
// Number of parameters samples used to smooth.
|
||||
const size_t kNumSamplesToSmooth = 20;
|
||||
|
||||
bool IsClockEstimationExperimentEnabled() {
|
||||
return webrtc::field_trial::IsEnabled("WebRTC-ClockEstimation");
|
||||
}
|
||||
|
||||
// Calculates the RTP timestamp frequency from two pairs of NTP/RTP timestamps.
|
||||
bool CalculateFrequency(int64_t ntp_ms1,
|
||||
@ -50,28 +43,6 @@ bool Contains(const std::list<RtpToNtpEstimator::RtcpMeasurement>& measurements,
|
||||
}
|
||||
} // namespace
|
||||
|
||||
bool RtpToNtpEstimator::Parameters::operator<(const Parameters& other) const {
|
||||
if (frequency_khz < other.frequency_khz - 1e-6) {
|
||||
return true;
|
||||
} else if (frequency_khz > other.frequency_khz + 1e-6) {
|
||||
return false;
|
||||
} else {
|
||||
return offset_ms < other.offset_ms - 1e-6;
|
||||
}
|
||||
}
|
||||
|
||||
bool RtpToNtpEstimator::Parameters::operator==(const Parameters& other) const {
|
||||
return !(other < *this || *this < other);
|
||||
}
|
||||
|
||||
bool RtpToNtpEstimator::Parameters::operator!=(const Parameters& other) const {
|
||||
return other < *this || *this < other;
|
||||
}
|
||||
|
||||
bool RtpToNtpEstimator::Parameters::operator<=(const Parameters& other) const {
|
||||
return !(other < *this);
|
||||
}
|
||||
|
||||
RtpToNtpEstimator::RtcpMeasurement::RtcpMeasurement(uint32_t ntp_secs,
|
||||
uint32_t ntp_frac,
|
||||
int64_t unwrapped_timestamp)
|
||||
@ -88,18 +59,13 @@ bool RtpToNtpEstimator::RtcpMeasurement::IsEqual(
|
||||
|
||||
// Class for converting an RTP timestamp to the NTP domain.
|
||||
RtpToNtpEstimator::RtpToNtpEstimator()
|
||||
: consecutive_invalid_samples_(0),
|
||||
smoothing_filter_(kNumSamplesToSmooth),
|
||||
params_calculated_(false),
|
||||
is_experiment_enabled_(IsClockEstimationExperimentEnabled()) {}
|
||||
|
||||
: consecutive_invalid_samples_(0), params_calculated_(false) {}
|
||||
RtpToNtpEstimator::~RtpToNtpEstimator() {}
|
||||
|
||||
void RtpToNtpEstimator::UpdateParameters() {
|
||||
if (measurements_.size() != kNumRtcpReportsToUse)
|
||||
return;
|
||||
|
||||
Parameters params;
|
||||
int64_t timestamp_new = measurements_.front().unwrapped_rtp_timestamp;
|
||||
int64_t timestamp_old = measurements_.back().unwrapped_rtp_timestamp;
|
||||
|
||||
@ -107,16 +73,11 @@ void RtpToNtpEstimator::UpdateParameters() {
|
||||
int64_t ntp_ms_old = measurements_.back().ntp_time.ToMs();
|
||||
|
||||
if (!CalculateFrequency(ntp_ms_new, timestamp_new, ntp_ms_old, timestamp_old,
|
||||
¶ms.frequency_khz)) {
|
||||
¶ms_.frequency_khz)) {
|
||||
return;
|
||||
}
|
||||
params.offset_ms = timestamp_new - params.frequency_khz * ntp_ms_new;
|
||||
params_.offset_ms = timestamp_new - params_.frequency_khz * ntp_ms_new;
|
||||
params_calculated_ = true;
|
||||
if (is_experiment_enabled_) {
|
||||
smoothing_filter_.Insert(params);
|
||||
} else {
|
||||
params_ = params;
|
||||
}
|
||||
}
|
||||
|
||||
bool RtpToNtpEstimator::UpdateMeasurements(uint32_t ntp_secs,
|
||||
@ -133,7 +94,6 @@ bool RtpToNtpEstimator::UpdateMeasurements(uint32_t ntp_secs,
|
||||
// RTCP SR report already added.
|
||||
return true;
|
||||
}
|
||||
|
||||
if (!new_measurement.ntp_time.Valid())
|
||||
return false;
|
||||
|
||||
@ -162,7 +122,6 @@ bool RtpToNtpEstimator::UpdateMeasurements(uint32_t ntp_secs,
|
||||
RTC_LOG(LS_WARNING) << "Multiple consecutively invalid RTCP SR reports, "
|
||||
"clearing measurements.";
|
||||
measurements_.clear();
|
||||
smoothing_filter_.Reset();
|
||||
params_calculated_ = false;
|
||||
}
|
||||
consecutive_invalid_samples_ = 0;
|
||||
@ -186,15 +145,12 @@ bool RtpToNtpEstimator::Estimate(int64_t rtp_timestamp,
|
||||
|
||||
int64_t rtp_timestamp_unwrapped = unwrapper_.Unwrap(rtp_timestamp);
|
||||
|
||||
Parameters params =
|
||||
is_experiment_enabled_ ? smoothing_filter_.GetFilteredValue() : params_;
|
||||
|
||||
// params_calculated_ should not be true unless ms params.frequency_khz has
|
||||
// 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);
|
||||
RTC_DCHECK_NE(params_.frequency_khz, 0.0);
|
||||
double rtp_ms =
|
||||
(static_cast<double>(rtp_timestamp_unwrapped) - params.offset_ms) /
|
||||
params.frequency_khz +
|
||||
(static_cast<double>(rtp_timestamp_unwrapped) - params_.offset_ms) /
|
||||
params_.frequency_khz +
|
||||
0.5f;
|
||||
|
||||
if (rtp_ms < 0)
|
||||
@ -203,14 +159,4 @@ bool RtpToNtpEstimator::Estimate(int64_t rtp_timestamp,
|
||||
*rtp_timestamp_ms = rtp_ms;
|
||||
return true;
|
||||
}
|
||||
|
||||
const rtc::Optional<RtpToNtpEstimator::Parameters> RtpToNtpEstimator::params()
|
||||
const {
|
||||
rtc::Optional<Parameters> res;
|
||||
if (params_calculated_) {
|
||||
res.emplace(is_experiment_enabled_ ? smoothing_filter_.GetFilteredValue()
|
||||
: params_);
|
||||
}
|
||||
return res;
|
||||
}
|
||||
} // namespace webrtc
|
||||
|
Reference in New Issue
Block a user