Report timing frames info in GetStats.

webrtc/video/receive_statistics_proxy.cc

Index: webrtc/video/receive_statistics_proxy.cc
diff --git a/webrtc/video/receive_statistics_proxy.cc b/webrtc/video/receive_statistics_proxy.cc
index 61b2ad13987aae3afb41f7aed304480b07e9b81f..747e7eae475e6481760ed743958af022c9456599 100644
--- a/webrtc/video/receive_statistics_proxy.cc
+++ b/webrtc/video/receive_statistics_proxy.cc
@@ -371,6 +371,11 @@ VideoReceiveStream::Stats ReceiveStatisticsProxy::GetStats() const {
   stats_.decode_frame_rate = decode_fps_estimator_.Rate(now_ms).value_or(0);
   stats_.total_bitrate_bps =
       static_cast<int>(total_byte_tracker_.ComputeRate() * 8);
+  stats_.timing_frame_info = timing_frame_info_;
+  // Reset reported value to empty, so it will be always
+  // overwritten in |OnTimingFrameInfoUpdated|, thus allowing to store new
+  // value instead of reported one.
+  timing_frame_info_.reset();

[sprang_webrtc, 2017/06/28 15:31:41]

I think this is problematic, since the GetStats calls can come from several
places, not all of them are from the "desired" call site.
Could we break out the timing frame polling to a separate method for appending
that data? Then we have a way to explicitly reset it and don't need a mutable
member. We'd need on in VideoReceiveStream as well then

[ilnik, 2017/06/29 08:39:56]

I don't understand what do you mean. In the end, it still will be returned via
GetStats, won't it? And there are no parameters to GetStats to understand if
it's called from a 'desired' state.

One solution would be to not reset observed timing frame info at all and return
the 'longest' timing frame for whole call so far. Or we could just return the
latest one available. Some outliers may be missed this way.

[sprang_webrtc, 2017/06/30 21:38:20]

I was thinking of direct calls to ReceiveStatisticsProxy from within the native
stack. I've seen usages like that where rather than implement a new rate tracker
or add another callback for bitrate/framrate etc we just call the stats proxies.


It'd be silly if we lost data because we had calls from vie_encode or jitter
estimator or whatnot.

   return stats_;
 }
 
@@ -432,6 +437,17 @@ void ReceiveStatisticsProxy::OnFrameBufferTimingsUpdated(
                        "ssrc", stats_.ssrc);
 }
 
+void ReceiveStatisticsProxy::OnTimingFrameInfoUpdated(
+    const TimingFrameInfo& info) {
+  rtc::CritScope lock(&crit_);
+  // Only the frame which was processed the longest since the last
+  // GetStats() call is reported. Therefore, only single 'longest' frame is
+  // stored.
+  if (!timing_frame_info_ || info.IsLongerThan(*timing_frame_info_)) {
+    timing_frame_info_.emplace(info);
+  }
+}
+
 void ReceiveStatisticsProxy::RtcpPacketTypesCounterUpdated(
     uint32_t ssrc,
     const RtcpPacketTypeCounter& packet_counter) {