diff --git a/webrtc/modules/rtp_rtcp/include/remote_ntp_time_estimator.h b/webrtc/modules/rtp_rtcp/include/remote_ntp_time_estimator.h index 207e749a02..cab488f3ea 100644 --- a/webrtc/modules/rtp_rtcp/include/remote_ntp_time_estimator.h +++ b/webrtc/modules/rtp_rtcp/include/remote_ntp_time_estimator.h @@ -43,7 +43,7 @@ class RemoteNtpTimeEstimator { private: Clock* clock_; std::unique_ptr ts_extrapolator_; - RtcpList rtcp_list_; + RtcpMeasurements rtcp_list_; int64_t last_timing_log_ms_; RTC_DISALLOW_COPY_AND_ASSIGN(RemoteNtpTimeEstimator); }; diff --git a/webrtc/system_wrappers/include/rtp_to_ntp.h b/webrtc/system_wrappers/include/rtp_to_ntp.h index 0c91928626..866144116c 100644 --- a/webrtc/system_wrappers/include/rtp_to_ntp.h +++ b/webrtc/system_wrappers/include/rtp_to_ntp.h @@ -20,26 +20,47 @@ namespace webrtc { struct RtcpMeasurement { RtcpMeasurement(); RtcpMeasurement(uint32_t ntp_secs, uint32_t ntp_frac, uint32_t timestamp); + bool IsEqual(const RtcpMeasurement& other) const; + uint32_t ntp_secs; uint32_t ntp_frac; uint32_t rtp_timestamp; }; -typedef std::list RtcpList; +struct RtcpMeasurements { + RtcpMeasurements(); + ~RtcpMeasurements(); + bool Contains(const RtcpMeasurement& other) const; + bool IsValid(const RtcpMeasurement& other) const; + void UpdateParameters(); -// Updates |rtcp_list| with timestamps from the latest RTCP SR. + // Estimated parameters from RTP and NTP timestamp pairs in |list|. + struct RtpToNtpParameters { + double frequency_khz = 0.0; + double offset_ms = 0.0; + bool calculated = false; + }; + + std::list list; + RtpToNtpParameters params; +}; + +// Updates |list| in |rtcp_measurements| with timestamps from the RTCP SR. // |new_rtcp_sr| will be set to true if these are the timestamps which have -// never be added to |rtcp_list|. +// never be added to |list|. +// |rtcp_measurements.params| are estimated from the RTP and NTP timestamp pairs +// in the |list| when a new RTCP SR is inserted. bool UpdateRtcpList(uint32_t ntp_secs, uint32_t ntp_frac, uint32_t rtp_timestamp, - RtcpList* rtcp_list, + RtcpMeasurements* rtcp_measurements, bool* new_rtcp_sr); -// Converts an RTP timestamp to the NTP domain in milliseconds using two -// (RTP timestamp, NTP timestamp) pairs. -bool RtpToNtpMs(int64_t rtp_timestamp, const RtcpList& rtcp, - int64_t* timestamp_in_ms); +// Converts an RTP timestamp to the NTP domain in milliseconds using the +// estimated |rtcp_measurements.params|. +bool RtpToNtpMs(int64_t rtp_timestamp, + const RtcpMeasurements& rtcp_measurements, + int64_t* rtp_timestamp_in_ms); // Returns 1 there has been a forward wrap around, 0 if there has been no wrap // around and -1 if there has been a backwards wrap around (i.e. reordering). diff --git a/webrtc/system_wrappers/source/rtp_to_ntp.cc b/webrtc/system_wrappers/source/rtp_to_ntp.cc index 05091461d4..8290749f13 100644 --- a/webrtc/system_wrappers/source/rtp_to_ntp.cc +++ b/webrtc/system_wrappers/source/rtp_to_ntp.cc @@ -10,27 +10,17 @@ #include "webrtc/system_wrappers/include/rtp_to_ntp.h" +#include "webrtc/base/logging.h" #include "webrtc/system_wrappers/include/clock.h" -#include - namespace webrtc { - -RtcpMeasurement::RtcpMeasurement() - : ntp_secs(0), ntp_frac(0), rtp_timestamp(0) {} - -RtcpMeasurement::RtcpMeasurement(uint32_t ntp_secs, uint32_t ntp_frac, - uint32_t timestamp) - : ntp_secs(ntp_secs), ntp_frac(ntp_frac), rtp_timestamp(timestamp) {} - -// Calculates the RTP timestamp frequency from two pairs of NTP and RTP -// timestamps. -bool CalculateFrequency( - int64_t rtcp_ntp_ms1, - uint32_t rtp_timestamp1, - int64_t rtcp_ntp_ms2, - uint32_t rtp_timestamp2, - double* frequency_khz) { +namespace { +// Calculates the RTP timestamp frequency from two pairs of NTP/RTP timestamps. +bool CalculateFrequency(int64_t rtcp_ntp_ms1, + uint32_t rtp_timestamp1, + int64_t rtcp_ntp_ms2, + uint32_t rtp_timestamp2, + double* frequency_khz) { if (rtcp_ntp_ms1 <= rtcp_ntp_ms2) { return false; } @@ -44,7 +34,6 @@ bool CalculateFrequency( bool CompensateForWrapAround(uint32_t new_timestamp, uint32_t old_timestamp, int64_t* compensated_timestamp) { - assert(compensated_timestamp); int64_t wraps = CheckForWrapArounds(new_timestamp, old_timestamp); if (wraps < 0) { // Reordering, don't use this packet. @@ -53,39 +42,111 @@ bool CompensateForWrapAround(uint32_t new_timestamp, *compensated_timestamp = new_timestamp + (wraps << 32); return true; } +} // namespace +// Class holding RTP and NTP timestamp from a RTCP SR report. +RtcpMeasurement::RtcpMeasurement() + : ntp_secs(0), ntp_frac(0), rtp_timestamp(0) {} + +RtcpMeasurement::RtcpMeasurement(uint32_t ntp_secs, + uint32_t ntp_frac, + uint32_t timestamp) + : ntp_secs(ntp_secs), ntp_frac(ntp_frac), rtp_timestamp(timestamp) {} + +bool 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_secs == other.ntp_secs && ntp_frac == other.ntp_frac) || + (rtp_timestamp == other.rtp_timestamp); +} + +// Class holding list of RTP and NTP timestamp pairs. +RtcpMeasurements::RtcpMeasurements() {} +RtcpMeasurements::~RtcpMeasurements() {} + +bool RtcpMeasurements::Contains(const RtcpMeasurement& other) const { + for (const auto& it : list) { + if (it.IsEqual(other)) + return true; + } + return false; +} + +bool RtcpMeasurements::IsValid(const RtcpMeasurement& other) const { + if (other.ntp_secs == 0 && other.ntp_frac == 0) { + // Invalid or not defined. + return false; + } + int64_t ntp_ms_new = Clock::NtpToMs(other.ntp_secs, other.ntp_frac); + for (const auto& it : list) { + if (ntp_ms_new <= Clock::NtpToMs(it.ntp_secs, it.ntp_frac)) { + // Old report. + return false; + } + int64_t timestamp_new = other.rtp_timestamp; + if (!CompensateForWrapAround(timestamp_new, it.rtp_timestamp, + ×tamp_new)) { + return false; + } + if (timestamp_new <= it.rtp_timestamp) { + LOG(LS_WARNING) << "Newer RTCP SR report with older RTP timestamp."; + return false; + } + } + return true; +} + +void RtcpMeasurements::UpdateParameters() { + if (list.size() != 2) + return; + + int64_t timestamp_new = list.front().rtp_timestamp; + int64_t timestamp_old = list.back().rtp_timestamp; + if (!CompensateForWrapAround(timestamp_new, timestamp_old, ×tamp_new)) + return; + + int64_t ntp_ms_new = + Clock::NtpToMs(list.front().ntp_secs, list.front().ntp_frac); + int64_t ntp_ms_old = + Clock::NtpToMs(list.back().ntp_secs, list.back().ntp_frac); + + if (!CalculateFrequency(ntp_ms_new, timestamp_new, ntp_ms_old, timestamp_old, + ¶ms.frequency_khz)) { + return; + } + params.offset_ms = timestamp_new - params.frequency_khz * ntp_ms_new; + params.calculated = true; +} + +// Updates list holding NTP and RTP timestamp pairs. bool UpdateRtcpList(uint32_t ntp_secs, uint32_t ntp_frac, uint32_t rtp_timestamp, - RtcpList* rtcp_list, + RtcpMeasurements* rtcp_measurements, bool* new_rtcp_sr) { *new_rtcp_sr = false; - if (ntp_secs == 0 && ntp_frac == 0) { + + RtcpMeasurement measurement(ntp_secs, ntp_frac, rtp_timestamp); + if (rtcp_measurements->Contains(measurement)) { + // RTCP SR report already added. + return true; + } + + if (!rtcp_measurements->IsValid(measurement)) { + // Old report or invalid parameters. return false; } - RtcpMeasurement measurement; - measurement.ntp_secs = ntp_secs; - measurement.ntp_frac = ntp_frac; - measurement.rtp_timestamp = rtp_timestamp; + // Two RTCP SR reports are needed to map between RTP and NTP. + // More than two will not improve the mapping. + if (rtcp_measurements->list.size() == 2) + rtcp_measurements->list.pop_back(); - for (RtcpList::iterator it = rtcp_list->begin(); - it != rtcp_list->end(); ++it) { - if ((measurement.ntp_secs == (*it).ntp_secs && - measurement.ntp_frac == (*it).ntp_frac) || - (measurement.rtp_timestamp == (*it).rtp_timestamp)) { - // This RTCP has already been added to the list. - return true; - } - } - - // We need two RTCP SR reports to map between RTP and NTP. More than two will - // not improve the mapping. - if (rtcp_list->size() == 2) { - rtcp_list->pop_back(); - } - rtcp_list->push_front(measurement); + rtcp_measurements->list.push_front(measurement); *new_rtcp_sr = true; + + // List updated, calculate new parameters. + rtcp_measurements->UpdateParameters(); return true; } @@ -94,45 +155,26 @@ bool UpdateRtcpList(uint32_t ntp_secs, // |rtp_timestamp_in_ms|. This function compensates for wrap arounds in RTP // timestamps and returns false if it can't do the conversion due to reordering. bool RtpToNtpMs(int64_t rtp_timestamp, - const RtcpList& rtcp, + const RtcpMeasurements& rtcp, int64_t* rtp_timestamp_in_ms) { - if (rtcp.size() != 2) + if (!rtcp.params.calculated || rtcp.list.empty()) return false; - int64_t rtcp_ntp_ms_new = Clock::NtpToMs(rtcp.front().ntp_secs, - rtcp.front().ntp_frac); - int64_t rtcp_ntp_ms_old = Clock::NtpToMs(rtcp.back().ntp_secs, - rtcp.back().ntp_frac); - int64_t rtcp_timestamp_new = rtcp.front().rtp_timestamp; - int64_t rtcp_timestamp_old = rtcp.back().rtp_timestamp; - if (!CompensateForWrapAround(rtcp_timestamp_new, - rtcp_timestamp_old, - &rtcp_timestamp_new)) { - return false; - } - if (rtcp_timestamp_new < rtcp_timestamp_old) - return false; - - double freq_khz; - if (!CalculateFrequency(rtcp_ntp_ms_new, - rtcp_timestamp_new, - rtcp_ntp_ms_old, - rtcp_timestamp_old, - &freq_khz)) { - return false; - } - double offset = rtcp_timestamp_new - freq_khz * rtcp_ntp_ms_new; + uint32_t rtcp_timestamp_old = rtcp.list.back().rtp_timestamp; int64_t rtp_timestamp_unwrapped; if (!CompensateForWrapAround(rtp_timestamp, rtcp_timestamp_old, &rtp_timestamp_unwrapped)) { return false; } - double rtp_timestamp_ntp_ms = (static_cast(rtp_timestamp_unwrapped) - - offset) / freq_khz + 0.5f; - if (rtp_timestamp_ntp_ms < 0) { + + double rtp_timestamp_ms = + (static_cast(rtp_timestamp_unwrapped) - rtcp.params.offset_ms) / + rtcp.params.frequency_khz + + 0.5f; + if (rtp_timestamp_ms < 0) { return false; } - *rtp_timestamp_in_ms = rtp_timestamp_ntp_ms; + *rtp_timestamp_in_ms = rtp_timestamp_ms; return true; } diff --git a/webrtc/system_wrappers/source/rtp_to_ntp_unittest.cc b/webrtc/system_wrappers/source/rtp_to_ntp_unittest.cc index 5ba3353f43..f65a3cf82e 100644 --- a/webrtc/system_wrappers/source/rtp_to_ntp_unittest.cc +++ b/webrtc/system_wrappers/source/rtp_to_ntp_unittest.cc @@ -37,156 +37,237 @@ TEST(WrapAroundTests, BackwardWrap) { EXPECT_EQ(-1, CheckForWrapArounds(0xFFFF0000, 0x0000FFFF)); } -TEST(WrapAroundTests, OldRtcpWrapped) { - RtcpList rtcp; +TEST(WrapAroundTests, OldRtcpWrapped_OldRtpTimestamp) { + RtcpMeasurements rtcp; + bool new_sr; uint32_t ntp_sec = 0; - uint32_t ntp_frac = 0; + uint32_t ntp_frac = 1; uint32_t timestamp = 0; - rtcp.push_front(RtcpMeasurement(ntp_sec, ntp_frac, timestamp)); + EXPECT_TRUE(UpdateRtcpList(ntp_sec, ntp_frac, timestamp, &rtcp, &new_sr)); ntp_frac += kOneMsInNtpFrac; timestamp -= kTimestampTicksPerMs; - rtcp.push_front(RtcpMeasurement(ntp_sec, ntp_frac, timestamp)); - ntp_frac += kOneMsInNtpFrac; - timestamp -= kTimestampTicksPerMs; - int64_t timestamp_in_ms = -1; - // This expected to fail since it's highly unlikely that the older RTCP - // has a much smaller RTP timestamp than the newer. - EXPECT_FALSE(RtpToNtpMs(timestamp, rtcp, ×tamp_in_ms)); + // Expected to fail since the older RTCP has a smaller RTP timestamp than the + // newer (old:0, new:4294967206). + EXPECT_FALSE(UpdateRtcpList(ntp_sec, ntp_frac, timestamp, &rtcp, &new_sr)); } TEST(WrapAroundTests, NewRtcpWrapped) { - RtcpList rtcp; + RtcpMeasurements rtcp; + bool new_sr; uint32_t ntp_sec = 0; - uint32_t ntp_frac = 0; + uint32_t ntp_frac = 1; uint32_t timestamp = 0xFFFFFFFF; - rtcp.push_front(RtcpMeasurement(ntp_sec, ntp_frac, timestamp)); + EXPECT_TRUE(UpdateRtcpList(ntp_sec, ntp_frac, timestamp, &rtcp, &new_sr)); ntp_frac += kOneMsInNtpFrac; timestamp += kTimestampTicksPerMs; - rtcp.push_front(RtcpMeasurement(ntp_sec, ntp_frac, timestamp)); - int64_t timestamp_in_ms = -1; - EXPECT_TRUE(RtpToNtpMs(rtcp.back().rtp_timestamp, rtcp, ×tamp_in_ms)); + EXPECT_TRUE(UpdateRtcpList(ntp_sec, ntp_frac, timestamp, &rtcp, &new_sr)); + int64_t timestamp_ms = -1; + EXPECT_TRUE(RtpToNtpMs(rtcp.list.back().rtp_timestamp, rtcp, ×tamp_ms)); // Since this RTP packet has the same timestamp as the RTCP packet constructed // at time 0 it should be mapped to 0 as well. - EXPECT_EQ(0, timestamp_in_ms); + EXPECT_EQ(0, timestamp_ms); } TEST(WrapAroundTests, RtpWrapped) { - RtcpList rtcp; + RtcpMeasurements rtcp; + bool new_sr; uint32_t ntp_sec = 0; - uint32_t ntp_frac = 0; + uint32_t ntp_frac = 1; uint32_t timestamp = 0xFFFFFFFF - 2 * kTimestampTicksPerMs; - rtcp.push_front(RtcpMeasurement(ntp_sec, ntp_frac, timestamp)); + EXPECT_TRUE(UpdateRtcpList(ntp_sec, ntp_frac, timestamp, &rtcp, &new_sr)); ntp_frac += kOneMsInNtpFrac; timestamp += kTimestampTicksPerMs; - rtcp.push_front(RtcpMeasurement(ntp_sec, ntp_frac, timestamp)); - ntp_frac += kOneMsInNtpFrac; - timestamp += kTimestampTicksPerMs; - int64_t timestamp_in_ms = -1; - EXPECT_TRUE(RtpToNtpMs(timestamp, rtcp, ×tamp_in_ms)); + EXPECT_TRUE(UpdateRtcpList(ntp_sec, ntp_frac, timestamp, &rtcp, &new_sr)); + + int64_t timestamp_ms = -1; + EXPECT_TRUE(RtpToNtpMs(rtcp.list.back().rtp_timestamp, rtcp, ×tamp_ms)); // Since this RTP packet has the same timestamp as the RTCP packet constructed // at time 0 it should be mapped to 0 as well. - EXPECT_EQ(2, timestamp_in_ms); + EXPECT_EQ(0, timestamp_ms); + // Two kTimestampTicksPerMs advanced. + timestamp += kTimestampTicksPerMs; + EXPECT_TRUE(RtpToNtpMs(timestamp, rtcp, ×tamp_ms)); + EXPECT_EQ(2, timestamp_ms); + // Wrapped rtp. + timestamp += kTimestampTicksPerMs; + EXPECT_TRUE(RtpToNtpMs(timestamp, rtcp, ×tamp_ms)); + EXPECT_EQ(3, timestamp_ms); } TEST(WrapAroundTests, OldRtp_RtcpsWrapped) { - RtcpList rtcp; + RtcpMeasurements rtcp; + bool new_sr; uint32_t ntp_sec = 0; - uint32_t ntp_frac = 0; + uint32_t ntp_frac = 1; uint32_t timestamp = 0; - rtcp.push_front(RtcpMeasurement(ntp_sec, ntp_frac, timestamp)); + EXPECT_TRUE(UpdateRtcpList(ntp_sec, ntp_frac, timestamp, &rtcp, &new_sr)); ntp_frac += kOneMsInNtpFrac; timestamp += kTimestampTicksPerMs; - rtcp.push_front(RtcpMeasurement(ntp_sec, ntp_frac, timestamp)); - ntp_frac += kOneMsInNtpFrac; + EXPECT_TRUE(UpdateRtcpList(ntp_sec, ntp_frac, timestamp, &rtcp, &new_sr)); timestamp -= 2*kTimestampTicksPerMs; - int64_t timestamp_in_ms = -1; - EXPECT_FALSE(RtpToNtpMs(timestamp, rtcp, ×tamp_in_ms)); + int64_t timestamp_ms = -1; + EXPECT_FALSE(RtpToNtpMs(timestamp, rtcp, ×tamp_ms)); } TEST(WrapAroundTests, OldRtp_NewRtcpWrapped) { - RtcpList rtcp; + RtcpMeasurements rtcp; + bool new_sr; uint32_t ntp_sec = 0; - uint32_t ntp_frac = 0; + uint32_t ntp_frac = 1; uint32_t timestamp = 0xFFFFFFFF; - rtcp.push_front(RtcpMeasurement(ntp_sec, ntp_frac, timestamp)); + EXPECT_TRUE(UpdateRtcpList(ntp_sec, ntp_frac, timestamp, &rtcp, &new_sr)); ntp_frac += kOneMsInNtpFrac; timestamp += kTimestampTicksPerMs; - rtcp.push_front(RtcpMeasurement(ntp_sec, ntp_frac, timestamp)); - ntp_frac += kOneMsInNtpFrac; + EXPECT_TRUE(UpdateRtcpList(ntp_sec, ntp_frac, timestamp, &rtcp, &new_sr)); timestamp -= kTimestampTicksPerMs; - int64_t timestamp_in_ms = -1; - EXPECT_TRUE(RtpToNtpMs(timestamp, rtcp, ×tamp_in_ms)); + int64_t timestamp_ms = -1; + EXPECT_TRUE(RtpToNtpMs(timestamp, rtcp, ×tamp_ms)); // Constructed at the same time as the first RTCP and should therefore be // mapped to zero. - EXPECT_EQ(0, timestamp_in_ms); + EXPECT_EQ(0, timestamp_ms); } -TEST(WrapAroundTests, OldRtp_OldRtcpWrapped) { - RtcpList rtcp; +TEST(UpdateRtcpListTests, InjectRtcpSr) { + const uint32_t kNtpSec = 10; + const uint32_t kNtpFrac = 12345; + const uint32_t kTs = 0x12345678; + bool new_sr; + RtcpMeasurements rtcp; + EXPECT_TRUE(UpdateRtcpList(kNtpSec, kNtpFrac, kTs, &rtcp, &new_sr)); + EXPECT_TRUE(new_sr); + EXPECT_EQ(1u, rtcp.list.size()); + EXPECT_EQ(kNtpSec, rtcp.list.front().ntp_secs); + EXPECT_EQ(kNtpFrac, rtcp.list.front().ntp_frac); + EXPECT_EQ(kTs, rtcp.list.front().rtp_timestamp); + // Add second report. + EXPECT_TRUE(UpdateRtcpList(kNtpSec, kNtpFrac + kOneMsInNtpFrac, kTs + 1, + &rtcp, &new_sr)); + EXPECT_EQ(2u, rtcp.list.size()); + EXPECT_EQ(kTs + 1, rtcp.list.front().rtp_timestamp); + EXPECT_EQ(kTs + 0, rtcp.list.back().rtp_timestamp); + // List should contain last two reports. + EXPECT_TRUE(UpdateRtcpList(kNtpSec, kNtpFrac + 2 * kOneMsInNtpFrac, kTs + 2, + &rtcp, &new_sr)); + EXPECT_EQ(2u, rtcp.list.size()); + EXPECT_EQ(kTs + 2, rtcp.list.front().rtp_timestamp); + EXPECT_EQ(kTs + 1, rtcp.list.back().rtp_timestamp); +} + +TEST(UpdateRtcpListTests, FailsForZeroNtp) { + RtcpMeasurements rtcp; uint32_t ntp_sec = 0; uint32_t ntp_frac = 0; - uint32_t timestamp = 0; - rtcp.push_front(RtcpMeasurement(ntp_sec, ntp_frac, timestamp)); - ntp_frac += kOneMsInNtpFrac; - timestamp -= kTimestampTicksPerMs; - rtcp.push_front(RtcpMeasurement(ntp_sec, ntp_frac, timestamp)); - ntp_frac += kOneMsInNtpFrac; - timestamp += 2*kTimestampTicksPerMs; - int64_t timestamp_in_ms = -1; - EXPECT_FALSE(RtpToNtpMs(timestamp, rtcp, ×tamp_in_ms)); -} - -TEST(RtpToNtpTests, FailsForDecreasingRtpTimestamp) { - const uint32_t kNtpSec1 = 3683354930; - const uint32_t kNtpFrac1 = 699925050; - const uint32_t kTimestamp1 = 2192705742; - const uint32_t kNtpSec2 = kNtpSec1; - const uint32_t kNtpFrac2 = kNtpFrac1 + kOneMsInNtpFrac; - const uint32_t kTimestamp2 = kTimestamp1 - kTimestampTicksPerMs; - RtcpList rtcp; - rtcp.push_front(RtcpMeasurement(kNtpSec1, kNtpFrac1, kTimestamp1)); - rtcp.push_front(RtcpMeasurement(kNtpSec2, kNtpFrac2, kTimestamp2)); - int64_t timestamp_in_ms = -1; - EXPECT_FALSE(RtpToNtpMs(kTimestamp1, rtcp, ×tamp_in_ms)); -} - -TEST(UpdateRtcpListTests, InjectRtcpSrWithEqualNtp) { - RtcpList rtcp; - uint32_t ntp_sec = 0; - uint32_t ntp_frac = 2; uint32_t timestamp = 0x12345678; + bool new_sr; + EXPECT_FALSE(UpdateRtcpList(ntp_sec, ntp_frac, timestamp, &rtcp, &new_sr)); + EXPECT_FALSE(new_sr); + EXPECT_EQ(0u, rtcp.list.size()); +} +TEST(UpdateRtcpListTests, FailsForEqualNtp) { + RtcpMeasurements rtcp; + uint32_t ntp_sec = 0; + uint32_t ntp_frac = 699925050; + uint32_t timestamp = 0x12345678; bool new_sr; EXPECT_TRUE(UpdateRtcpList(ntp_sec, ntp_frac, timestamp, &rtcp, &new_sr)); EXPECT_TRUE(new_sr); - + EXPECT_EQ(1u, rtcp.list.size()); + // Ntp time already added, list not updated. ++timestamp; EXPECT_TRUE(UpdateRtcpList(ntp_sec, ntp_frac, timestamp, &rtcp, &new_sr)); EXPECT_FALSE(new_sr); + EXPECT_EQ(1u, rtcp.list.size()); } -TEST(UpdateRtcpListTests, InjectRtcpSrWithEqualTimestamp) { - RtcpList rtcp; - uint32_t ntp_sec = 0; - uint32_t ntp_frac = 2; +TEST(UpdateRtcpListTests, FailsForOldNtp) { + RtcpMeasurements rtcp; + uint32_t ntp_sec = 1; + uint32_t ntp_frac = 699925050; uint32_t timestamp = 0x12345678; - bool new_sr; EXPECT_TRUE(UpdateRtcpList(ntp_sec, ntp_frac, timestamp, &rtcp, &new_sr)); EXPECT_TRUE(new_sr); + EXPECT_EQ(1u, rtcp.list.size()); + // Old ntp time, list not updated. + ntp_frac -= kOneMsInNtpFrac; + timestamp += kTimestampTicksPerMs; + EXPECT_FALSE(UpdateRtcpList(ntp_sec, ntp_frac, timestamp, &rtcp, &new_sr)); + EXPECT_EQ(1u, rtcp.list.size()); +} +TEST(UpdateRtcpListTests, FailsForEqualTimestamp) { + RtcpMeasurements rtcp; + uint32_t ntp_sec = 0; + uint32_t ntp_frac = 2; + uint32_t timestamp = 0x12345678; + bool new_sr; + EXPECT_TRUE(UpdateRtcpList(ntp_sec, ntp_frac, timestamp, &rtcp, &new_sr)); + EXPECT_TRUE(new_sr); + EXPECT_EQ(1u, rtcp.list.size()); + // Timestamp already added, list not updated. ++ntp_frac; EXPECT_TRUE(UpdateRtcpList(ntp_sec, ntp_frac, timestamp, &rtcp, &new_sr)); EXPECT_FALSE(new_sr); + EXPECT_EQ(1u, rtcp.list.size()); } -TEST(UpdateRtcpListTests, InjectRtcpSrWithZeroNtpFails) { - RtcpList rtcp; +TEST(UpdateRtcpListTests, FailsForOldRtpTimestamp) { + RtcpMeasurements rtcp; uint32_t ntp_sec = 0; - uint32_t ntp_frac = 0; + uint32_t ntp_frac = 2; uint32_t timestamp = 0x12345678; - bool new_sr; + EXPECT_TRUE(UpdateRtcpList(ntp_sec, ntp_frac, timestamp, &rtcp, &new_sr)); + EXPECT_TRUE(new_sr); + EXPECT_EQ(1u, rtcp.list.size()); + // Old timestamp, list not updated. + ntp_frac += kOneMsInNtpFrac; + timestamp -= kTimestampTicksPerMs; EXPECT_FALSE(UpdateRtcpList(ntp_sec, ntp_frac, timestamp, &rtcp, &new_sr)); + EXPECT_FALSE(new_sr); + EXPECT_EQ(1u, rtcp.list.size()); } + +TEST(UpdateRtcpListTests, VerifyParameters) { + RtcpMeasurements rtcp; + uint32_t ntp_sec = 1; + uint32_t ntp_frac = 2; + uint32_t timestamp = 0x12345678; + bool new_sr; + EXPECT_TRUE(UpdateRtcpList(ntp_sec, ntp_frac, timestamp, &rtcp, &new_sr)); + EXPECT_TRUE(new_sr); + EXPECT_FALSE(rtcp.params.calculated); + // Add second report, parameters should be calculated. + ntp_frac += kOneMsInNtpFrac; + timestamp += kTimestampTicksPerMs; + EXPECT_TRUE(UpdateRtcpList(ntp_sec, ntp_frac, timestamp, &rtcp, &new_sr)); + EXPECT_TRUE(rtcp.params.calculated); + EXPECT_DOUBLE_EQ(90.0, rtcp.params.frequency_khz); + EXPECT_NE(0.0, rtcp.params.offset_ms); +} + +TEST(RtpToNtpTests, FailsForEmptyList) { + RtcpMeasurements rtcp; + rtcp.params.calculated = true; + // List is empty, conversion of RTP to NTP time should fail. + EXPECT_EQ(0u, rtcp.list.size()); + int64_t timestamp_ms = -1; + EXPECT_FALSE(RtpToNtpMs(0, rtcp, ×tamp_ms)); +} + +TEST(RtpToNtpTests, FailsForNoParameters) { + RtcpMeasurements rtcp; + uint32_t ntp_sec = 1; + uint32_t ntp_frac = 2; + uint32_t timestamp = 0x12345678; + bool new_sr; + EXPECT_TRUE(UpdateRtcpList(ntp_sec, ntp_frac, timestamp, &rtcp, &new_sr)); + EXPECT_EQ(1u, rtcp.list.size()); + // Parameters are not calculated, conversion of RTP to NTP time should fail. + EXPECT_FALSE(rtcp.params.calculated); + int64_t timestamp_ms = -1; + EXPECT_FALSE(RtpToNtpMs(timestamp, rtcp, ×tamp_ms)); +} + }; // namespace webrtc diff --git a/webrtc/video/receive_statistics_proxy.cc b/webrtc/video/receive_statistics_proxy.cc index e7310e8b19..ded510a55e 100644 --- a/webrtc/video/receive_statistics_proxy.cc +++ b/webrtc/video/receive_statistics_proxy.cc @@ -18,6 +18,10 @@ #include "webrtc/system_wrappers/include/metrics.h" namespace webrtc { +namespace { +// Periodic time interval for processing samples for |freq_offset_counter_|. +const int64_t kFreqOffsetProcessIntervalMs = 40000; +} // namespace ReceiveStatisticsProxy::ReceiveStatisticsProxy( const VideoReceiveStream::Config* config, @@ -29,7 +33,8 @@ ReceiveStatisticsProxy::ReceiveStatisticsProxy( decode_fps_estimator_(1000, 1000), renders_fps_estimator_(1000, 1000), render_fps_tracker_(100, 10u), - render_pixel_tracker_(100, 10u) { + render_pixel_tracker_(100, 10u), + freq_offset_counter_(clock, nullptr, kFreqOffsetProcessIntervalMs) { stats_.ssrc = config_.rtp.remote_ssrc; for (auto it : config_.rtp.rtx) rtx_stats_[it.second.ssrc] = StreamDataCounters(); @@ -68,6 +73,11 @@ void ReceiveStatisticsProxy::UpdateHistograms() { if (sync_offset_ms != -1) { RTC_HISTOGRAM_COUNTS_10000("WebRTC.Video.AVSyncOffsetInMs", sync_offset_ms); } + AggregatedStats freq_offset_stats = freq_offset_counter_.GetStats(); + if (freq_offset_stats.num_samples > 0) { + RTC_HISTOGRAM_COUNTS_10000("WebRTC.Video.RtpToNtpFreqOffsetInKhz", + freq_offset_stats.average); + } int qp = qp_counters_.vp8.Avg(kMinRequiredSamples); if (qp != -1) @@ -276,10 +286,19 @@ void ReceiveStatisticsProxy::OnRenderedFrame(const VideoFrame& frame) { } } -void ReceiveStatisticsProxy::OnSyncOffsetUpdated(int64_t sync_offset_ms) { +void ReceiveStatisticsProxy::OnSyncOffsetUpdated(int64_t sync_offset_ms, + double estimated_freq_khz) { rtc::CritScope lock(&crit_); sync_offset_counter_.Add(std::abs(sync_offset_ms)); stats_.sync_offset_ms = sync_offset_ms; + + const double kMaxFreqKhz = 10000.0; + int offset_khz = kMaxFreqKhz; + // Should not be zero or negative. If so, report max. + if (estimated_freq_khz < kMaxFreqKhz && estimated_freq_khz > 0.0) + offset_khz = static_cast(std::fabs(estimated_freq_khz - 90.0) + 0.5); + + freq_offset_counter_.Add(offset_khz); } void ReceiveStatisticsProxy::OnReceiveRatesUpdated(uint32_t bitRate, diff --git a/webrtc/video/receive_statistics_proxy.h b/webrtc/video/receive_statistics_proxy.h index 04777716af..445a731f24 100644 --- a/webrtc/video/receive_statistics_proxy.h +++ b/webrtc/video/receive_statistics_proxy.h @@ -22,6 +22,7 @@ #include "webrtc/common_video/include/frame_callback.h" #include "webrtc/modules/video_coding/include/video_coding_defines.h" #include "webrtc/video/report_block_stats.h" +#include "webrtc/video/stats_counter.h" #include "webrtc/video/video_stream_decoder.h" #include "webrtc/video_receive_stream.h" @@ -44,7 +45,7 @@ class ReceiveStatisticsProxy : public VCMReceiveStatisticsCallback, VideoReceiveStream::Stats GetStats() const; void OnDecodedFrame(); - void OnSyncOffsetUpdated(int64_t sync_offset_ms); + void OnSyncOffsetUpdated(int64_t sync_offset_ms, double estimated_freq_khz); void OnRenderedFrame(const VideoFrame& frame); void OnIncomingPayloadType(int payload_type); void OnDecoderImplementationName(const char* implementation_name); @@ -121,6 +122,7 @@ class ReceiveStatisticsProxy : public VCMReceiveStatisticsCallback, SampleCounter current_delay_counter_ GUARDED_BY(crit_); SampleCounter delay_counter_ GUARDED_BY(crit_); SampleCounter e2e_delay_counter_ GUARDED_BY(crit_); + MaxCounter freq_offset_counter_ GUARDED_BY(crit_); ReportBlockStats report_block_stats_ GUARDED_BY(crit_); QpCounters qp_counters_; // Only accessed on the decoding thread. std::map rtx_stats_ GUARDED_BY(crit_); diff --git a/webrtc/video/receive_statistics_proxy_unittest.cc b/webrtc/video/receive_statistics_proxy_unittest.cc index 894ee8fe73..b8c4b20f99 100644 --- a/webrtc/video/receive_statistics_proxy_unittest.cc +++ b/webrtc/video/receive_statistics_proxy_unittest.cc @@ -12,9 +12,13 @@ #include +#include "webrtc/system_wrappers/include/metrics_default.h" #include "webrtc/test/gtest.h" namespace webrtc { +namespace { +const int64_t kFreqOffsetProcessIntervalInMs = 40000; +} // namespace // TODO(sakal): ReceiveStatisticsProxy is lacking unittesting. class ReceiveStatisticsProxyTest : public ::testing::Test { @@ -24,6 +28,7 @@ class ReceiveStatisticsProxyTest : public ::testing::Test { protected: virtual void SetUp() { + metrics::Reset(); statistics_proxy_.reset(new ReceiveStatisticsProxy(&config_, &fake_clock_)); } @@ -33,8 +38,8 @@ class ReceiveStatisticsProxyTest : public ::testing::Test { } SimulatedClock fake_clock_; + const VideoReceiveStream::Config config_; std::unique_ptr statistics_proxy_; - VideoReceiveStream::Config config_; }; TEST_F(ReceiveStatisticsProxyTest, OnDecodedFrameIncreasesFramesDecoded) { @@ -45,4 +50,23 @@ TEST_F(ReceiveStatisticsProxyTest, OnDecodedFrameIncreasesFramesDecoded) { } } +TEST_F(ReceiveStatisticsProxyTest, RtpToNtpFrequencyOffsetHistogramIsUpdated) { + const int64_t kSyncOffsetMs = 22; + const double kFreqKhz = 90.0; + statistics_proxy_->OnSyncOffsetUpdated(kSyncOffsetMs, kFreqKhz); + statistics_proxy_->OnSyncOffsetUpdated(kSyncOffsetMs, kFreqKhz + 2.2); + fake_clock_.AdvanceTimeMilliseconds(kFreqOffsetProcessIntervalInMs); + // Process interval passed, max diff: 2. + statistics_proxy_->OnSyncOffsetUpdated(kSyncOffsetMs, kFreqKhz + 1.1); + statistics_proxy_->OnSyncOffsetUpdated(kSyncOffsetMs, kFreqKhz - 4.2); + statistics_proxy_->OnSyncOffsetUpdated(kSyncOffsetMs, kFreqKhz - 0.9); + fake_clock_.AdvanceTimeMilliseconds(kFreqOffsetProcessIntervalInMs); + // Process interval passed, max diff: 4. + statistics_proxy_->OnSyncOffsetUpdated(kSyncOffsetMs, kFreqKhz); + statistics_proxy_.reset(); + // Average reported: (2 + 4) / 2 = 3. + EXPECT_EQ(1, metrics::NumSamples("WebRTC.Video.RtpToNtpFreqOffsetInKhz")); + EXPECT_EQ(1, metrics::NumEvents("WebRTC.Video.RtpToNtpFreqOffsetInKhz", 3)); +} + } // namespace webrtc diff --git a/webrtc/video/rtp_streams_synchronizer.cc b/webrtc/video/rtp_streams_synchronizer.cc index 885dad3979..3bc208fe99 100644 --- a/webrtc/video/rtp_streams_synchronizer.cc +++ b/webrtc/video/rtp_streams_synchronizer.cc @@ -40,8 +40,8 @@ int UpdateMeasurements(StreamSynchronization::Measurements* stream, } bool new_rtcp_sr = false; - if (!UpdateRtcpList( - ntp_secs, ntp_frac, rtp_timestamp, &stream->rtcp, &new_rtcp_sr)) { + if (!UpdateRtcpList(ntp_secs, ntp_frac, rtp_timestamp, &stream->rtcp, + &new_rtcp_sr)) { return -1; } @@ -168,7 +168,9 @@ void RtpStreamsSynchronizer::Process() { } bool RtpStreamsSynchronizer::GetStreamSyncOffsetInMs( - const VideoFrame& frame, int64_t* stream_offset_ms) const { + const VideoFrame& frame, + int64_t* stream_offset_ms, + double* estimated_freq_khz) const { rtc::CritScope lock(&crit_); if (voe_channel_id_ == -1) return false; @@ -197,6 +199,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_.rtcp.params.frequency_khz; return true; } diff --git a/webrtc/video/rtp_streams_synchronizer.h b/webrtc/video/rtp_streams_synchronizer.h index 082bec7b6e..bc24d6f807 100644 --- a/webrtc/video/rtp_streams_synchronizer.h +++ b/webrtc/video/rtp_streams_synchronizer.h @@ -46,8 +46,11 @@ class RtpStreamsSynchronizer : public Module { // Gets the sync offset between the current played out audio frame and the // video |frame|. Returns true on success, false otherwise. + // The estimated frequency is the frequency used in the RTP to NTP timestamp + // conversion. bool GetStreamSyncOffsetInMs(const VideoFrame& frame, - int64_t* stream_offset_ms) const; + int64_t* stream_offset_ms, + double* estimated_freq_khz) const; private: Clock* const clock_; diff --git a/webrtc/video/stream_synchronization.h b/webrtc/video/stream_synchronization.h index f231cfb175..c700ee3bf7 100644 --- a/webrtc/video/stream_synchronization.h +++ b/webrtc/video/stream_synchronization.h @@ -22,7 +22,7 @@ class StreamSynchronization { public: struct Measurements { Measurements() : rtcp(), latest_receive_time_ms(0), latest_timestamp(0) {} - RtcpList rtcp; + RtcpMeasurements rtcp; int64_t latest_receive_time_ms; uint32_t latest_timestamp; }; diff --git a/webrtc/video/stream_synchronization_unittest.cc b/webrtc/video/stream_synchronization_unittest.cc index f09bee4ae7..d328d6af43 100644 --- a/webrtc/video/stream_synchronization_unittest.cc +++ b/webrtc/video/stream_synchronization_unittest.cc @@ -98,24 +98,30 @@ class StreamSynchronizationTest : public ::testing::Test { int audio_offset = 0; int video_frequency = static_cast(kDefaultVideoFrequency * video_clock_drift_ + 0.5); + bool new_sr; int video_offset = 0; StreamSynchronization::Measurements audio; StreamSynchronization::Measurements video; // Generate NTP/RTP timestamp pair for both streams corresponding to RTCP. - audio.rtcp.push_front(send_time_->GenerateRtcp(audio_frequency, - audio_offset)); + RtcpMeasurement rtcp = + send_time_->GenerateRtcp(audio_frequency, audio_offset); + EXPECT_TRUE(UpdateRtcpList(rtcp.ntp_secs, rtcp.ntp_frac, rtcp.rtp_timestamp, + &audio.rtcp, &new_sr)); send_time_->IncreaseTimeMs(100); receive_time_->IncreaseTimeMs(100); - video.rtcp.push_front(send_time_->GenerateRtcp(video_frequency, - video_offset)); + rtcp = send_time_->GenerateRtcp(video_frequency, video_offset); + EXPECT_TRUE(UpdateRtcpList(rtcp.ntp_secs, rtcp.ntp_frac, rtcp.rtp_timestamp, + &video.rtcp, &new_sr)); send_time_->IncreaseTimeMs(900); receive_time_->IncreaseTimeMs(900); - audio.rtcp.push_front(send_time_->GenerateRtcp(audio_frequency, - audio_offset)); + rtcp = send_time_->GenerateRtcp(audio_frequency, audio_offset); + EXPECT_TRUE(UpdateRtcpList(rtcp.ntp_secs, rtcp.ntp_frac, rtcp.rtp_timestamp, + &audio.rtcp, &new_sr)); send_time_->IncreaseTimeMs(100); receive_time_->IncreaseTimeMs(100); - video.rtcp.push_front(send_time_->GenerateRtcp(video_frequency, - video_offset)); + rtcp = send_time_->GenerateRtcp(video_frequency, video_offset); + EXPECT_TRUE(UpdateRtcpList(rtcp.ntp_secs, rtcp.ntp_frac, rtcp.rtp_timestamp, + &video.rtcp, &new_sr)); send_time_->IncreaseTimeMs(900); receive_time_->IncreaseTimeMs(900); diff --git a/webrtc/video/video_receive_stream.cc b/webrtc/video/video_receive_stream.cc index 7d191c8179..27d1fe4614 100644 --- a/webrtc/video/video_receive_stream.cc +++ b/webrtc/video/video_receive_stream.cc @@ -346,13 +346,15 @@ void VideoReceiveStream::OnFrame(const VideoFrame& video_frame) { stats_proxy_.OnDecodedFrame(); int64_t sync_offset_ms; + double estimated_freq_khz; // TODO(tommi): GetStreamSyncOffsetInMs grabs three locks. One inside the // function itself, another in GetChannel() and a third in // GetPlayoutTimestamp. Seems excessive. Anyhow, I'm assuming the function // succeeds most of the time, which leads to grabbing a fourth lock. - if (rtp_stream_sync_.GetStreamSyncOffsetInMs(video_frame, &sync_offset_ms)) { + if (rtp_stream_sync_.GetStreamSyncOffsetInMs(video_frame, &sync_offset_ms, + &estimated_freq_khz)) { // TODO(tommi): OnSyncOffsetUpdated grabs a lock. - stats_proxy_.OnSyncOffsetUpdated(sync_offset_ms); + stats_proxy_.OnSyncOffsetUpdated(sync_offset_ms, estimated_freq_khz); } // config_.renderer must never be null if we're getting this callback.