More gracefully handle rtp timestamp jumps in the rtp to ntp estimator.
BUG=webrtc:7905
Review-Url: https://codereview.webrtc.org/2963133003
Cr-Commit-Position: refs/heads/master@{#18860}
diff --git a/webrtc/system_wrappers/include/rtp_to_ntp_estimator.h b/webrtc/system_wrappers/include/rtp_to_ntp_estimator.h
index a17428b..9d6da05 100644
--- a/webrtc/system_wrappers/include/rtp_to_ntp_estimator.h
+++ b/webrtc/system_wrappers/include/rtp_to_ntp_estimator.h
@@ -54,9 +54,12 @@
const Parameters& params() const { return params_; }
+ static const int kMaxInvalidSamples = 3;
+
private:
void UpdateParameters();
+ int consecutive_invalid_samples_;
std::list<RtcpMeasurement> measurements_;
Parameters params_;
};
diff --git a/webrtc/system_wrappers/source/rtp_to_ntp_estimator.cc b/webrtc/system_wrappers/source/rtp_to_ntp_estimator.cc
index 339635e..baa218d 100644
--- a/webrtc/system_wrappers/source/rtp_to_ntp_estimator.cc
+++ b/webrtc/system_wrappers/source/rtp_to_ntp_estimator.cc
@@ -54,30 +54,6 @@
}
return false;
}
-
-bool IsValid(const std::list<RtpToNtpEstimator::RtcpMeasurement>& measurements,
- const RtpToNtpEstimator::RtcpMeasurement& other) {
- if (!other.ntp_time.Valid())
- return false;
-
- int64_t ntp_ms_new = other.ntp_time.ToMs();
- for (const auto& measurement : measurements) {
- if (ntp_ms_new <= measurement.ntp_time.ToMs()) {
- // Old report.
- return false;
- }
- int64_t timestamp_new = other.rtp_timestamp;
- if (!CompensateForWrapAround(timestamp_new, measurement.rtp_timestamp,
- ×tamp_new)) {
- return false;
- }
- if (timestamp_new <= measurement.rtp_timestamp) {
- LOG(LS_WARNING) << "Newer RTCP SR report with older RTP timestamp.";
- return false;
- }
- }
- return true;
-}
} // namespace
RtpToNtpEstimator::RtcpMeasurement::RtcpMeasurement(uint32_t ntp_secs,
@@ -93,7 +69,7 @@
}
// Class for converting an RTP timestamp to the NTP domain.
-RtpToNtpEstimator::RtpToNtpEstimator() {}
+RtpToNtpEstimator::RtpToNtpEstimator() : consecutive_invalid_samples_(0) {}
RtpToNtpEstimator::~RtpToNtpEstimator() {}
void RtpToNtpEstimator::UpdateParameters() {
@@ -122,21 +98,51 @@
bool* new_rtcp_sr) {
*new_rtcp_sr = false;
- RtcpMeasurement measurement(ntp_secs, ntp_frac, rtp_timestamp);
- if (Contains(measurements_, measurement)) {
+ RtcpMeasurement new_measurement(ntp_secs, ntp_frac, rtp_timestamp);
+ if (Contains(measurements_, new_measurement)) {
// RTCP SR report already added.
return true;
}
- if (!IsValid(measurements_, measurement)) {
- // Old report or invalid parameters.
+ if (!new_measurement.ntp_time.Valid())
return false;
+
+ 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.
+ 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) {
+ LOG(LS_WARNING)
+ << "Newer RTCP SR report with older RTP timestamp, dropping";
+ invalid_sample = true;
+ break;
+ }
}
+ if (invalid_sample) {
+ ++consecutive_invalid_samples_;
+ if (consecutive_invalid_samples_ < kMaxInvalidSamples) {
+ return false;
+ }
+ LOG(LS_WARNING) << "Multiple consecutively invalid RTCP SR reports, "
+ "clearing measurements.";
+ measurements_.clear();
+ }
+ consecutive_invalid_samples_ = 0;
// Insert new RTCP SR report.
if (measurements_.size() == kNumRtcpReportsToUse)
measurements_.pop_back();
- measurements_.push_front(measurement);
+ measurements_.push_front(new_measurement);
*new_rtcp_sr = true;
// List updated, calculate new parameters.
diff --git a/webrtc/system_wrappers/source/rtp_to_ntp_estimator_unittest.cc b/webrtc/system_wrappers/source/rtp_to_ntp_estimator_unittest.cc
index 09b47b4..cfcb13b 100644
--- a/webrtc/system_wrappers/source/rtp_to_ntp_estimator_unittest.cc
+++ b/webrtc/system_wrappers/source/rtp_to_ntp_estimator_unittest.cc
@@ -138,6 +138,48 @@
EXPECT_EQ(0, timestamp_ms);
}
+TEST(WrapAroundTests, GracefullyHandleRtpJump) {
+ RtpToNtpEstimator estimator;
+ bool new_sr;
+ uint32_t ntp_sec = 0;
+ uint32_t ntp_frac = 1;
+ uint32_t timestamp = 0;
+ EXPECT_TRUE(
+ estimator.UpdateMeasurements(ntp_sec, ntp_frac, timestamp, &new_sr));
+ ntp_frac += kOneMsInNtpFrac;
+ timestamp += kTimestampTicksPerMs;
+ EXPECT_TRUE(
+ estimator.UpdateMeasurements(ntp_sec, ntp_frac, timestamp, &new_sr));
+ ntp_frac += kOneMsInNtpFrac;
+ timestamp -= kTimestampTicksPerMs;
+ int64_t timestamp_ms = -1;
+ EXPECT_TRUE(estimator.Estimate(timestamp, ×tamp_ms));
+ // Constructed at the same time as the first RTCP and should therefore be
+ // mapped to zero.
+ EXPECT_EQ(0, timestamp_ms);
+
+ timestamp -= 0xFFFFF;
+ for (int i = 0; i < RtpToNtpEstimator::kMaxInvalidSamples - 1; ++i) {
+ EXPECT_FALSE(
+ estimator.UpdateMeasurements(ntp_sec, ntp_frac, timestamp, &new_sr));
+ ntp_frac += kOneMsInNtpFrac;
+ timestamp += kTimestampTicksPerMs;
+ }
+ EXPECT_TRUE(
+ estimator.UpdateMeasurements(ntp_sec, ntp_frac, timestamp, &new_sr));
+ ntp_frac += kOneMsInNtpFrac;
+ timestamp += kTimestampTicksPerMs;
+ EXPECT_TRUE(
+ estimator.UpdateMeasurements(ntp_sec, ntp_frac, timestamp, &new_sr));
+ ntp_frac += kOneMsInNtpFrac;
+ timestamp += kTimestampTicksPerMs;
+
+ timestamp_ms = -1;
+ EXPECT_TRUE(estimator.Estimate(timestamp, ×tamp_ms));
+ // 6 milliseconds has passed since the start of the test.
+ EXPECT_EQ(6, timestamp_ms);
+}
+
TEST(UpdateRtcpMeasurementTests, FailsForZeroNtp) {
RtpToNtpEstimator estimator;
uint32_t ntp_sec = 0;
