Add average rtt to CallStatsObserver and an average rtt histogram.
TBR=mflodman@webrtc.org
BUG=webrtc:4711,webrtc:4548
Review URL: https://codereview.webrtc.org/1279543005
Cr-Commit-Position: refs/heads/master@{#9687}
diff --git a/webrtc/modules/interface/module_common_types.h b/webrtc/modules/interface/module_common_types.h
index 232e695..02ce03f 100644
--- a/webrtc/modules/interface/module_common_types.h
+++ b/webrtc/modules/interface/module_common_types.h
@@ -440,7 +440,7 @@
// CallStats object using RegisterStatsObserver.
class CallStatsObserver {
public:
- virtual void OnRttUpdate(int64_t rtt_ms) = 0;
+ virtual void OnRttUpdate(int64_t avg_rtt_ms, int64_t max_rtt_ms) = 0;
virtual ~CallStatsObserver() {}
};
diff --git a/webrtc/modules/remote_bitrate_estimator/remote_bitrate_estimator_abs_send_time.cc b/webrtc/modules/remote_bitrate_estimator/remote_bitrate_estimator_abs_send_time.cc
index c5faf16..2050ea1 100644
--- a/webrtc/modules/remote_bitrate_estimator/remote_bitrate_estimator_abs_send_time.cc
+++ b/webrtc/modules/remote_bitrate_estimator/remote_bitrate_estimator_abs_send_time.cc
@@ -372,9 +372,10 @@
}
}
-void RemoteBitrateEstimatorAbsSendTime::OnRttUpdate(int64_t rtt) {
+void RemoteBitrateEstimatorAbsSendTime::OnRttUpdate(int64_t avg_rtt_ms,
+ int64_t max_rtt_ms) {
CriticalSectionScoped cs(crit_sect_.get());
- remote_rate_.SetRtt(rtt);
+ remote_rate_.SetRtt(avg_rtt_ms);
}
void RemoteBitrateEstimatorAbsSendTime::RemoveStream(unsigned int ssrc) {
diff --git a/webrtc/modules/remote_bitrate_estimator/remote_bitrate_estimator_abs_send_time.h b/webrtc/modules/remote_bitrate_estimator/remote_bitrate_estimator_abs_send_time.h
index 0d3802a..1bee6bd 100644
--- a/webrtc/modules/remote_bitrate_estimator/remote_bitrate_estimator_abs_send_time.h
+++ b/webrtc/modules/remote_bitrate_estimator/remote_bitrate_estimator_abs_send_time.h
@@ -83,7 +83,7 @@
// deleted.
int32_t Process() override;
int64_t TimeUntilNextProcess() override;
- void OnRttUpdate(int64_t rtt) override;
+ void OnRttUpdate(int64_t avg_rtt_ms, int64_t max_rtt_ms) override;
void RemoveStream(unsigned int ssrc) override;
bool LatestEstimate(std::vector<unsigned int>* ssrcs,
unsigned int* bitrate_bps) const override;
diff --git a/webrtc/modules/remote_bitrate_estimator/remote_bitrate_estimator_single_stream.cc b/webrtc/modules/remote_bitrate_estimator/remote_bitrate_estimator_single_stream.cc
index 762a317..6fd54e9 100644
--- a/webrtc/modules/remote_bitrate_estimator/remote_bitrate_estimator_single_stream.cc
+++ b/webrtc/modules/remote_bitrate_estimator/remote_bitrate_estimator_single_stream.cc
@@ -182,9 +182,10 @@
}
}
-void RemoteBitrateEstimatorSingleStream::OnRttUpdate(int64_t rtt) {
+void RemoteBitrateEstimatorSingleStream::OnRttUpdate(int64_t avg_rtt_ms,
+ int64_t max_rtt_ms) {
CriticalSectionScoped cs(crit_sect_.get());
- remote_rate_->SetRtt(rtt);
+ remote_rate_->SetRtt(avg_rtt_ms);
}
void RemoteBitrateEstimatorSingleStream::RemoveStream(unsigned int ssrc) {
diff --git a/webrtc/modules/remote_bitrate_estimator/remote_bitrate_estimator_single_stream.h b/webrtc/modules/remote_bitrate_estimator/remote_bitrate_estimator_single_stream.h
index 0432355..5ca2100 100644
--- a/webrtc/modules/remote_bitrate_estimator/remote_bitrate_estimator_single_stream.h
+++ b/webrtc/modules/remote_bitrate_estimator/remote_bitrate_estimator_single_stream.h
@@ -34,7 +34,7 @@
bool was_paced) override;
int32_t Process() override;
int64_t TimeUntilNextProcess() override;
- void OnRttUpdate(int64_t rtt) override;
+ void OnRttUpdate(int64_t avg_rtt_ms, int64_t max_rtt_ms) override;
void RemoveStream(unsigned int ssrc) override;
bool LatestEstimate(std::vector<unsigned int>* ssrcs,
unsigned int* bitrate_bps) const override;
diff --git a/webrtc/modules/remote_bitrate_estimator/test/estimators/remb.cc b/webrtc/modules/remote_bitrate_estimator/test/estimators/remb.cc
index f0d57ad..8ec2b72 100644
--- a/webrtc/modules/remote_bitrate_estimator/test/estimators/remb.cc
+++ b/webrtc/modules/remote_bitrate_estimator/test/estimators/remb.cc
@@ -77,7 +77,7 @@
ss << "Estimate_" << flow_id_ << "#1";
estimate_log_prefix_ = ss.str();
// Default RTT in RemoteRateControl is 200 ms ; 50 ms is more realistic.
- estimator_->OnRttUpdate(50);
+ estimator_->OnRttUpdate(50, 50);
}
RembReceiver::~RembReceiver() {
diff --git a/webrtc/modules/remote_bitrate_estimator/test/estimators/send_side.cc b/webrtc/modules/remote_bitrate_estimator/test/estimators/send_side.cc
index b003d45..1f95f65 100644
--- a/webrtc/modules/remote_bitrate_estimator/test/estimators/send_side.cc
+++ b/webrtc/modules/remote_bitrate_estimator/test/estimators/send_side.cc
@@ -61,7 +61,7 @@
int64_t rtt_ms =
clock_->TimeInMilliseconds() - feedback.latest_send_time_ms();
- rbe_->OnRttUpdate(rtt_ms);
+ rbe_->OnRttUpdate(rtt_ms, rtt_ms);
BWE_TEST_LOGGING_PLOT(1, "RTT", clock_->TimeInMilliseconds(), rtt_ms);
rbe_->IncomingPacketFeedbackVector(packet_feedback_vector);
diff --git a/webrtc/video_engine/call_stats.cc b/webrtc/video_engine/call_stats.cc
index 6e51eeb..92855e3 100644
--- a/webrtc/video_engine/call_stats.cc
+++ b/webrtc/video_engine/call_stats.cc
@@ -123,7 +123,7 @@
if (max_rtt_ms_ > 0) {
for (std::list<CallStatsObserver*>::iterator it = observers_.begin();
it != observers_.end(); ++it) {
- (*it)->OnRttUpdate(max_rtt_ms_);
+ (*it)->OnRttUpdate(avg_rtt_ms_, max_rtt_ms_);
}
}
return 0;
diff --git a/webrtc/video_engine/call_stats_unittest.cc b/webrtc/video_engine/call_stats_unittest.cc
index 0febbd0..bfe52da 100644
--- a/webrtc/video_engine/call_stats_unittest.cc
+++ b/webrtc/video_engine/call_stats_unittest.cc
@@ -27,7 +27,7 @@
MockStatsObserver() {}
virtual ~MockStatsObserver() {}
- MOCK_METHOD1(OnRttUpdate, void(int64_t));
+ MOCK_METHOD2(OnRttUpdate, void(int64_t, int64_t));
};
class CallStatsTest : public ::testing::Test {
@@ -48,15 +48,13 @@
const int64_t kRtt = 25;
rtcp_rtt_stats->OnRttUpdate(kRtt);
- EXPECT_CALL(stats_observer, OnRttUpdate(kRtt))
- .Times(1);
+ EXPECT_CALL(stats_observer, OnRttUpdate(kRtt, kRtt)).Times(1);
call_stats_->Process();
EXPECT_EQ(kRtt, rtcp_rtt_stats->LastProcessedRtt());
const int64_t kRttTimeOutMs = 1500 + 10;
TickTime::AdvanceFakeClock(kRttTimeOutMs);
- EXPECT_CALL(stats_observer, OnRttUpdate(_))
- .Times(0);
+ EXPECT_CALL(stats_observer, OnRttUpdate(_, _)).Times(0);
call_stats_->Process();
EXPECT_EQ(0, rtcp_rtt_stats->LastProcessedRtt());
@@ -70,27 +68,23 @@
rtcp_rtt_stats->OnRttUpdate(100);
// Time isn't updated yet.
- EXPECT_CALL(stats_observer, OnRttUpdate(_))
- .Times(0);
+ EXPECT_CALL(stats_observer, OnRttUpdate(_, _)).Times(0);
call_stats_->Process();
// Advance clock and verify we get an update.
TickTime::AdvanceFakeClock(1000);
- EXPECT_CALL(stats_observer, OnRttUpdate(_))
- .Times(1);
+ EXPECT_CALL(stats_observer, OnRttUpdate(_, _)).Times(1);
call_stats_->Process();
// Advance clock just too little to get an update.
TickTime::AdvanceFakeClock(999);
rtcp_rtt_stats->OnRttUpdate(100);
- EXPECT_CALL(stats_observer, OnRttUpdate(_))
- .Times(0);
+ EXPECT_CALL(stats_observer, OnRttUpdate(_, _)).Times(0);
call_stats_->Process();
// Advance enough to trigger a new update.
TickTime::AdvanceFakeClock(1);
- EXPECT_CALL(stats_observer, OnRttUpdate(_))
- .Times(1);
+ EXPECT_CALL(stats_observer, OnRttUpdate(_, _)).Times(1);
call_stats_->Process();
call_stats_->DeregisterStatsObserver(&stats_observer);
@@ -113,10 +107,8 @@
// Verify both observers are updated.
TickTime::AdvanceFakeClock(1000);
- EXPECT_CALL(stats_observer_1, OnRttUpdate(kRtt))
- .Times(1);
- EXPECT_CALL(stats_observer_2, OnRttUpdate(kRtt))
- .Times(1);
+ EXPECT_CALL(stats_observer_1, OnRttUpdate(kRtt, kRtt)).Times(1);
+ EXPECT_CALL(stats_observer_2, OnRttUpdate(kRtt, kRtt)).Times(1);
call_stats_->Process();
// Deregister the second observer and verify update is only sent to the first
@@ -124,20 +116,16 @@
call_stats_->DeregisterStatsObserver(&stats_observer_2);
rtcp_rtt_stats->OnRttUpdate(kRtt);
TickTime::AdvanceFakeClock(1000);
- EXPECT_CALL(stats_observer_1, OnRttUpdate(kRtt))
- .Times(1);
- EXPECT_CALL(stats_observer_2, OnRttUpdate(kRtt))
- .Times(0);
+ EXPECT_CALL(stats_observer_1, OnRttUpdate(kRtt, kRtt)).Times(1);
+ EXPECT_CALL(stats_observer_2, OnRttUpdate(kRtt, kRtt)).Times(0);
call_stats_->Process();
// Deregister the first observer.
call_stats_->DeregisterStatsObserver(&stats_observer_1);
rtcp_rtt_stats->OnRttUpdate(kRtt);
TickTime::AdvanceFakeClock(1000);
- EXPECT_CALL(stats_observer_1, OnRttUpdate(kRtt))
- .Times(0);
- EXPECT_CALL(stats_observer_2, OnRttUpdate(kRtt))
- .Times(0);
+ EXPECT_CALL(stats_observer_1, OnRttUpdate(kRtt, kRtt)).Times(0);
+ EXPECT_CALL(stats_observer_2, OnRttUpdate(kRtt, kRtt)).Times(0);
call_stats_->Process();
}
@@ -153,16 +141,15 @@
// Set a first value and verify the callback is triggered.
const int64_t kFirstRtt = 100;
rtcp_rtt_stats->OnRttUpdate(kFirstRtt);
- EXPECT_CALL(stats_observer, OnRttUpdate(kFirstRtt))
- .Times(1);
+ EXPECT_CALL(stats_observer, OnRttUpdate(kFirstRtt, kFirstRtt)).Times(1);
call_stats_->Process();
// Increase rtt and verify the new value is reported.
TickTime::AdvanceFakeClock(1000);
const int64_t kHighRtt = kFirstRtt + 20;
+ const int64_t kAvgRtt1 = 103;
rtcp_rtt_stats->OnRttUpdate(kHighRtt);
- EXPECT_CALL(stats_observer, OnRttUpdate(kHighRtt))
- .Times(1);
+ EXPECT_CALL(stats_observer, OnRttUpdate(kAvgRtt1, kHighRtt)).Times(1);
call_stats_->Process();
// Increase time enough for a new update, but not too much to make the
@@ -170,16 +157,16 @@
// in the callback.
TickTime::AdvanceFakeClock(1000);
const int64_t kLowRtt = kFirstRtt - 20;
+ const int64_t kAvgRtt2 = 102;
rtcp_rtt_stats->OnRttUpdate(kLowRtt);
- EXPECT_CALL(stats_observer, OnRttUpdate(kHighRtt))
- .Times(1);
+ EXPECT_CALL(stats_observer, OnRttUpdate(kAvgRtt2, kHighRtt)).Times(1);
call_stats_->Process();
// Advance time to make the high report invalid, the lower rtt should now be
// in the callback.
TickTime::AdvanceFakeClock(1000);
- EXPECT_CALL(stats_observer, OnRttUpdate(kLowRtt))
- .Times(1);
+ const int64_t kAvgRtt3 = 95;
+ EXPECT_CALL(stats_observer, OnRttUpdate(kAvgRtt3, kLowRtt)).Times(1);
call_stats_->Process();
call_stats_->DeregisterStatsObserver(&stats_observer);
@@ -198,8 +185,7 @@
const int64_t kAvgRtt = 20;
rtcp_rtt_stats->OnRttUpdate(kRttLow);
rtcp_rtt_stats->OnRttUpdate(kRttHigh);
- EXPECT_CALL(stats_observer, OnRttUpdate(kRttHigh))
- .Times(1);
+ EXPECT_CALL(stats_observer, OnRttUpdate(kAvgRtt, kRttHigh)).Times(1);
call_stats_->Process();
EXPECT_EQ(kAvgRtt, rtcp_rtt_stats->LastProcessedRtt());
@@ -207,8 +193,7 @@
TickTime::AdvanceFakeClock(1000);
rtcp_rtt_stats->OnRttUpdate(kRttLow);
rtcp_rtt_stats->OnRttUpdate(kRttHigh);
- EXPECT_CALL(stats_observer, OnRttUpdate(kRttHigh))
- .Times(1);
+ EXPECT_CALL(stats_observer, OnRttUpdate(kAvgRtt, kRttHigh)).Times(1);
call_stats_->Process();
EXPECT_EQ(kAvgRtt, rtcp_rtt_stats->LastProcessedRtt());
diff --git a/webrtc/video_engine/vie_channel.cc b/webrtc/video_engine/vie_channel.cc
index df371ed..b8664ce 100644
--- a/webrtc/video_engine/vie_channel.cc
+++ b/webrtc/video_engine/vie_channel.cc
@@ -49,8 +49,8 @@
virtual ~ChannelStatsObserver() {}
// Implements StatsObserver.
- virtual void OnRttUpdate(int64_t rtt) {
- owner_->OnRttUpdate(rtt);
+ virtual void OnRttUpdate(int64_t avg_rtt_ms, int64_t max_rtt_ms) {
+ owner_->OnRttUpdate(avg_rtt_ms, max_rtt_ms);
}
private:
@@ -119,6 +119,9 @@
max_nack_reordering_threshold_(kMaxPacketAgeToNack),
pre_render_callback_(NULL),
report_block_stats_sender_(new ReportBlockStats()),
+ time_of_first_rtt_ms_(-1),
+ rtt_sum_ms_(0),
+ num_rtts_(0),
rtp_rtcp_modules_(
CreateRtpRtcpModules(ViEModuleId(engine_id_, channel_id_),
!sender,
@@ -198,6 +201,17 @@
void ViEChannel::UpdateHistograms() {
int64_t now = Clock::GetRealTimeClock()->TimeInMilliseconds();
+ {
+ CriticalSectionScoped cs(crit_.get());
+ int64_t elapsed_sec = (now - time_of_first_rtt_ms_) / 1000;
+ if (time_of_first_rtt_ms_ != -1 && num_rtts_ > 0 &&
+ elapsed_sec > metrics::kMinRunTimeInSeconds) {
+ int64_t avg_rtt_ms = (rtt_sum_ms_ + num_rtts_ / 2) / num_rtts_;
+ RTC_HISTOGRAM_COUNTS_10000(
+ "WebRTC.Video.AverageRoundTripTimeInMilliseconds", avg_rtt_ms);
+ }
+ }
+
if (sender_) {
RtcpPacketTypeCounter rtcp_counter;
GetSendRtcpPacketTypeCounter(&rtcp_counter);
@@ -1119,8 +1133,14 @@
return true;
}
-void ViEChannel::OnRttUpdate(int64_t rtt) {
- vcm_->SetReceiveChannelParameters(rtt);
+void ViEChannel::OnRttUpdate(int64_t avg_rtt_ms, int64_t max_rtt_ms) {
+ vcm_->SetReceiveChannelParameters(max_rtt_ms);
+
+ CriticalSectionScoped cs(crit_.get());
+ if (time_of_first_rtt_ms_ == -1)
+ time_of_first_rtt_ms_ = Clock::GetRealTimeClock()->TimeInMilliseconds();
+ rtt_sum_ms_ += avg_rtt_ms;
+ ++num_rtts_;
}
int ViEChannel::ProtectionRequest(const FecProtectionParams* delta_fec_params,
diff --git a/webrtc/video_engine/vie_channel.h b/webrtc/video_engine/vie_channel.h
index 6c5782e..85b18cf 100644
--- a/webrtc/video_engine/vie_channel.h
+++ b/webrtc/video_engine/vie_channel.h
@@ -318,7 +318,7 @@
static bool ChannelDecodeThreadFunction(void* obj);
bool ChannelDecodeProcess();
- void OnRttUpdate(int64_t rtt);
+ void OnRttUpdate(int64_t avg_rtt_ms, int64_t max_rtt_ms);
int ProtectionRequest(const FecProtectionParams* delta_fec_params,
const FecProtectionParams* key_fec_params,
@@ -488,6 +488,10 @@
const rtc::scoped_ptr<ReportBlockStats> report_block_stats_sender_;
+ int64_t time_of_first_rtt_ms_ GUARDED_BY(crit_);
+ int64_t rtt_sum_ms_ GUARDED_BY(crit_);
+ size_t num_rtts_ GUARDED_BY(crit_);
+
// RtpRtcp modules, declared last as they use other members on construction.
const std::vector<RtpRtcp*> rtp_rtcp_modules_;
size_t num_active_rtp_rtcp_modules_ GUARDED_BY(crit_);
diff --git a/webrtc/video_engine/vie_channel_group.cc b/webrtc/video_engine/vie_channel_group.cc
index cf6b19c..b1a21f0 100644
--- a/webrtc/video_engine/vie_channel_group.cc
+++ b/webrtc/video_engine/vie_channel_group.cc
@@ -70,9 +70,9 @@
return rbe_->TimeUntilNextProcess();
}
- void OnRttUpdate(int64_t rtt) override {
+ void OnRttUpdate(int64_t avg_rtt_ms, int64_t max_rtt_ms) override {
CriticalSectionScoped cs(crit_sect_.get());
- rbe_->OnRttUpdate(rtt);
+ rbe_->OnRttUpdate(avg_rtt_ms, max_rtt_ms);
}
void RemoveStream(unsigned int ssrc) override {
