RTCMediaStreamTrackStats.frameWidth/Height,framesPerSecond,framesDecoded

webrtc/api/rtcstatscollector.cc

Index: webrtc/api/rtcstatscollector.cc
diff --git a/webrtc/api/rtcstatscollector.cc b/webrtc/api/rtcstatscollector.cc
index c54619ef5d189e77723c0f540c79a73977668bd3..0747c8d2761ac560e0106a41cfeec33d00837069 100644
--- a/webrtc/api/rtcstatscollector.cc
+++ b/webrtc/api/rtcstatscollector.cc
@@ -446,6 +446,12 @@ void ProduceMediaStreamAndTrackStats(
           if (sender_info) {
             video_track_stats->ssrc_ids = StringSsrcsFromSsrcs(
                 sender_info->ssrcs());
+            video_track_stats->frame_width = static_cast<uint32_t>(
+                sender_info->send_frame_width);
+            video_track_stats->frame_height = static_cast<uint32_t>(
+                sender_info->send_frame_height);
+            video_track_stats->frames_per_second = static_cast<double>(
+                sender_info->framerate_sent);

[hbos, 2016/12/29 12:45:41]

Alternatively, VideoSenderInfo has framerate_input too.

[Taylor Brandstetter, 2017/01/04 23:25:23]

I'm not completely sure that framerate_sent (or framerate_input) are correct.
Suppose the source framerate is 60fps, the target encode framerate is 30fps, and
the actual number of frames encoded during the last second is 29 (because one
had to be dropped). Should frames_per_second be 60 (framerate_input), 29
(framerate_sent), or 30 (which might be this?
https://cs.chromium.org/chromium/src/third_party/webrtc/modules/video_coding/...).

We probably should loop in someone more familiar with the video pipeline.

           }
         } else {
           const cricket::VideoReceiverInfo* receiver_info =
@@ -453,20 +459,18 @@ void ProduceMediaStreamAndTrackStats(
           if (receiver_info) {
             video_track_stats->ssrc_ids = StringSsrcsFromSsrcs(
                 receiver_info->ssrcs());
+            video_track_stats->frame_width = static_cast<uint32_t>(
+                receiver_info->frame_width);
+            video_track_stats->frame_height = static_cast<uint32_t>(
+                receiver_info->frame_height);
+            video_track_stats->frames_per_second = static_cast<double>(
+                receiver_info->framerate_output);

[hbos, 2016/12/29 12:45:41]

Alternatively, VideoReceiverInfo also has framerate_rcvd, framerate_decoded,
framerate_render_input and framerate_render_output.

[Taylor Brandstetter, 2017/01/04 23:25:23]

I'm also not sure that this is completely right. For codecs that store the
framerate in the bitstream (such as H.264), should this be the framerate from
the bitstream, or the framerate received?

Here's where the former would be for H.264:
https://cs.chromium.org/chromium/src/third_party/ffmpeg/libavcodec/avcodec.h?...

It looks like we don't currently do anything with it. So for now, framerate_rcvd
may be the best we can do.

+            video_track_stats->frames_decoded = receiver_info->frames_decoded;

[hbos, 2016/12/29 15:29:28]

Uhm, this is the same as RTCInboundRTPStreamStats.framesDecoded added in
https://codereview.webrtc.org/2588373005/diff/20001/webrtc/api/rtcstatscollec....

That can't be right, duplicate stats?

[hta-webrtc, 2017/01/02 16:06:42]

Add a TODO to count the frames delivered to the track instead. I think this
needs some redesign, since in the case where we switch what SSRC we take input
from (simulcast), we need to keep the count correct over the switch, which
requires saving the number at the moment of the switch for both the track and
for the SSRC we switch to, which requires having memory for that in the
statscollector.

This needs deadbeef or video team input: When we have incoming simulcast, do we
switch SSRCs, or does the switch mux everything into a single SSRC? If separate
SSRCs, do we even decode the ones that aren't being shown?

[Taylor Brandstetter, 2017/01/04 23:25:23]

I believe this already does count the frames delivered to the track. When using
simulcast, the entire simulcast group is represented by a single VideoSenderInfo
with multiple SSRCs (again, using the first one for identification). Which means
that we're not currently collecting per-simulcast-layer stats, which (if they
existed) would go in SsrcSenderInfo.

           }
         }
       }
       video_track_stats->remote_source = !is_local;
       video_track_stats->detached = false;
-      if (video_track->GetSource()) {
-        VideoTrackSourceInterface::Stats video_track_source_stats;
-        if (video_track->GetSource()->GetStats(&video_track_source_stats)) {
-          video_track_stats->frame_width = static_cast<uint32_t>(
-              video_track_source_stats.input_width);
-          video_track_stats->frame_height = static_cast<uint32_t>(
-              video_track_source_stats.input_height);

[hbos, 2016/12/29 12:45:41]

These sizes correspond to googFrame[Width/Height]Input, probably not as
interesting as googFrame[Width/Height]Sent
(sender_info->send_frame_[width/height])?

[hbos, 2016/12/29 14:24:48]

I also found out that this was not available in the rtcstats_integrationtest.cc
for remoteSource = true which made it not possible to distinguish between remote
video and audio by looking at the frameWidth.

-        }
-      }
       report->AddStats(std::move(video_track_stats));
     }
     report->AddStats(std::move(stream_stats));