RTCCodecStats added

webrtc/api/rtcstatscollector.cc

Index: webrtc/api/rtcstatscollector.cc
diff --git a/webrtc/api/rtcstatscollector.cc b/webrtc/api/rtcstatscollector.cc
index fbc6dbc5d3be86ce60aec2fa44beb579f318159c..66bb2ab3ad81fc52e823fac02a4183019d773ff0 100644
--- a/webrtc/api/rtcstatscollector.cc
+++ b/webrtc/api/rtcstatscollector.cc
@@ -33,6 +33,16 @@ std::string RTCCertificateIDFromFingerprint(const std::string& fingerprint) {
   return "RTCCertificate_" + fingerprint;
 }
 
+std::string RTCCodecStatsIDFromDirectionTypeAndPayload(

[Taylor Brandstetter, 2016/11/17 22:01:51]

nit: I think you meant "DirectionAndPayloadType"?

[hta-webrtc, 2016/11/23 07:37:35]

It's from "Direction, Type and Payload", so I guess it's OK. The alternate names
are "DirectionTypePayload" and "DirectionAndTypeAndPayload". Not much
readability improvement in either.

[hbos, 2016/11/23 09:40:59]

Hm, I replaced "Type" with "Media" as in media type.

+    bool inbound, bool audio, uint32_t payload_type) {
+  if (inbound) {
+    return audio ? "RTCCodec_InboundAudio_" + rtc::ToString<>(payload_type)

[Taylor Brandstetter, 2016/11/17 22:01:51]

This is close, but what's really needed is something to differentiate the
transport being used, not "audio vs video". For example, if you're not bundling,
this is possible:

m=video 9 UDP/TLS/RTP/SAVPF 100
a=mid:VideoTrack1
a=rtpmap:100 CodecA
...
m=video:9 UDP/TLS/RTP/SAVPF 100
a=mid:VideoTrack2
a=rtpmap:100 CodecB

The same payload type refers to different codecs, which is possible if the
tracks are using different transports and thus different RTP sessions.

When I say "something to differentiate the transport being used", the transport
ID in "proxy_to_transport" should be good.

[hbos, 2016/11/18 09:08:13]

Hmm. I do pc_->session()->voice_channel()->GetStats(...) and
pc_->session()->video_channel()->GetStats(...).

The session() has different transports, and e.g. GetTransportStats performs
transport_controller_->GetStats with the voice, video and data channels'
transport names so each channel can use a different transport (but this gives
other stats than codecs).

But in any given time we only have one voice, one video and one data channel in
this context (std::unique_ptr)? I can set up voice to use one transport and
video to use another, but I'm already distinguishing between audio and video.
Are there other channels equivalent of session()->blah_channel() that I'm
ignoring right now?

For me to test this scenario I need to be able to add multiple
Mock[Voice/Video]Channels but I can only mock the session to return a single
voice/video channel.

I could make the codec ID be based on transport name instead of "Audio" or
"Video", that might be more correct, but both are today unique unless I'm
mistaken.

Does this mean that there are advertised codecs that these stats ignore because
they're not part of the currently selected channels? Should codecs stats come
from transport stats instead? Or is it correct to only return codecs stats that
are on existing channels (disregarding things mentioned in the SDP)?



Oh and there are also data channels, are there data codecs (that are not audio
or video) that should be included?

[Taylor Brandstetter, 2016/11/18 19:34:15]

No, you're right. This is what I was talking about when I said "this issue is
impossible to run into until we implement 'Unified Plan' SDP". I guess there's
no way to test it right now, either.
No, what you have now should be good. A "BaseChannel" is roughly equivalent to
an m= section in SDP, which is roughly equivalent to an RtpTransceiver.
Nope, don't need to worry about that.

[hta-webrtc, 2016/11/23 07:37:35]

Add a TODO to update this when we handle multiple m= lines of the same media
type, and we're good.

[hbos, 2016/11/23 09:40:59]

Done.

+                 : "RTCCodec_InboundVideo_" + rtc::ToString<>(payload_type);
+  }
+  return audio ? "RTCCodec_OutboundAudio_" + rtc::ToString<>(payload_type)
+               : "RTCCodec_OutboundVideo_" + rtc::ToString<>(payload_type);
+}
+
 std::string RTCIceCandidatePairStatsIDFromConnectionInfo(
     const cricket::ConnectionInfo& info) {
   return "RTCIceCandidatePair_" + info.local_candidate.id() + "_" +
@@ -100,6 +110,21 @@ const char* DataStateToRTCDataChannelState(
   }
 }
 
+std::unique_ptr<RTCCodecStats> CodecStatsFromRtpCodecParameters(
+    uint64_t timestamp_us, bool inbound, bool audio,
+    const RtpCodecParameters& codec_params) {
+  RTC_DCHECK_GE(codec_params.payload_type, 0);
+  RTC_DCHECK_LE(codec_params.payload_type, 127);
+  uint32_t payload_type = static_cast<uint32_t>(codec_params.payload_type);
+  std::unique_ptr<RTCCodecStats> codec_stats(new RTCCodecStats(
+      RTCCodecStatsIDFromDirectionTypeAndPayload(inbound, audio, payload_type),
+      timestamp_us));
+  codec_stats->payload_type = payload_type;
+  codec_stats->codec = (audio ? "audio/" : "video/") + codec_params.mime_type;
+  codec_stats->clock_rate = static_cast<uint32_t>(codec_params.clock_rate);
+  return codec_stats;
+}
+
 void SetMediaStreamTrackStatsFromMediaStreamTrackInterface(
     const MediaStreamTrackInterface& track,
     RTCMediaStreamTrackStats* track_stats) {
@@ -392,14 +417,17 @@ void RTCStatsCollector::ProducePartialResultsOnSignalingThread(
   SessionStats session_stats;
   if (pc_->session()->GetTransportStats(&session_stats)) {
     std::map<std::string, CertificateStatsPair> transport_cert_stats =
-        PrepareTransportCertificateStats_s(session_stats);
+        PrepareTransportCertificateStats(session_stats);
+    MediaInfo media_info = PrepareMediaInfo(session_stats);
 
     ProduceCertificateStats_s(
         timestamp_us, transport_cert_stats, report.get());
+    ProduceCodecStats_s(
+        timestamp_us, media_info, report.get());
     ProduceIceCandidateAndPairStats_s(
         timestamp_us, session_stats, report.get());
     ProduceRTPStreamStats_s(
-        timestamp_us, session_stats, report.get());
+        timestamp_us, session_stats, media_info, report.get());
     ProduceTransportStats_s(
         timestamp_us, session_stats, transport_cert_stats, report.get());
   }
@@ -501,6 +529,38 @@ void RTCStatsCollector::ProduceCertificateStats_s(
   }
 }
 
+void RTCStatsCollector::ProduceCodecStats_s(
+    int64_t timestamp_us, const MediaInfo& media_info,
+    RTCStatsReport* report) const {
+  RTC_DCHECK(signaling_thread_->IsCurrent());
+  // Audio
+  if (media_info.voice) {
+    // Inbound
+    for (const auto& pair : media_info.voice->receive_codecs) {
+      report->AddStats(CodecStatsFromRtpCodecParameters(
+          timestamp_us, true, true, pair.second));
+    }
+    // Outbound
+    for (const auto& pair : media_info.voice->send_codecs) {
+      report->AddStats(CodecStatsFromRtpCodecParameters(
+          timestamp_us, false, true, pair.second));
+    }
+  }
+  // Video
+  if (media_info.video) {
+    // Inbound
+    for (const auto& pair : media_info.video->receive_codecs) {
+      report->AddStats(CodecStatsFromRtpCodecParameters(
+          timestamp_us, true, false, pair.second));
+    }
+    // Outbound
+    for (const auto& pair : media_info.video->send_codecs) {
+      report->AddStats(CodecStatsFromRtpCodecParameters(
+          timestamp_us, false, false, pair.second));
+    }
+  }
+}
+
 void RTCStatsCollector::ProduceDataChannelStats_s(
     int64_t timestamp_us, RTCStatsReport* report) const {
   RTC_DCHECK(signaling_thread_->IsCurrent());
@@ -592,93 +652,107 @@ void RTCStatsCollector::ProducePeerConnectionStats_s(
 
 void RTCStatsCollector::ProduceRTPStreamStats_s(
     int64_t timestamp_us, const SessionStats& session_stats,
-    RTCStatsReport* report) const {
+    const MediaInfo& media_info, RTCStatsReport* report) const {
   RTC_DCHECK(signaling_thread_->IsCurrent());
 
   // Audio
-  if (pc_->session()->voice_channel()) {
-    cricket::VoiceMediaInfo voice_media_info;
-    if (pc_->session()->voice_channel()->GetStats(&voice_media_info)) {
-      std::string transport_id = RTCTransportStatsIDFromBaseChannel(
-          session_stats.proxy_to_transport, *pc_->session()->voice_channel());
-      RTC_DCHECK(!transport_id.empty());
-      // Inbound
-      for (const cricket::VoiceReceiverInfo& voice_receiver_info :
-           voice_media_info.receivers) {
-        // TODO(nisse): SSRC == 0 currently means none. Delete check when that
-        // is fixed.
-        if (voice_receiver_info.ssrc() == 0)
-          continue;
-        std::unique_ptr<RTCInboundRTPStreamStats> inbound_audio(
-            new RTCInboundRTPStreamStats(
-                RTCInboundRTPStreamStatsIDFromSSRC(
-                    true, voice_receiver_info.ssrc()),
-                timestamp_us));
-        SetInboundRTPStreamStatsFromVoiceReceiverInfo(
-            voice_receiver_info, inbound_audio.get());
-        inbound_audio->transport_id = transport_id;
-        report->AddStats(std::move(inbound_audio));
+  if (media_info.voice) {
+    std::string transport_id = RTCTransportStatsIDFromBaseChannel(
+        session_stats.proxy_to_transport, *pc_->session()->voice_channel());
+    RTC_DCHECK(!transport_id.empty());
+    // Inbound
+    for (const cricket::VoiceReceiverInfo& voice_receiver_info :
+         media_info.voice->receivers) {
+      // TODO(nisse): SSRC == 0 currently means none. Delete check when that
+      // is fixed.
+      if (voice_receiver_info.ssrc() == 0)
+        continue;
+      std::unique_ptr<RTCInboundRTPStreamStats> inbound_audio(
+          new RTCInboundRTPStreamStats(
+              RTCInboundRTPStreamStatsIDFromSSRC(
+                  true, voice_receiver_info.ssrc()),
+              timestamp_us));
+      SetInboundRTPStreamStatsFromVoiceReceiverInfo(
+          voice_receiver_info, inbound_audio.get());
+      inbound_audio->transport_id = transport_id;
+      if (voice_receiver_info.codec_payload_type) {
+        inbound_audio->codec_id =
+            RTCCodecStatsIDFromDirectionTypeAndPayload(
+                true, true, *voice_receiver_info.codec_payload_type);
       }
-      // Outbound
-      for (const cricket::VoiceSenderInfo& voice_sender_info :
-           voice_media_info.senders) {
-        // TODO(nisse): SSRC == 0 currently means none. Delete check when that
-        // is fixed.
-        if (voice_sender_info.ssrc() == 0)
-          continue;
-        std::unique_ptr<RTCOutboundRTPStreamStats> outbound_audio(
-            new RTCOutboundRTPStreamStats(
-                RTCOutboundRTPStreamStatsIDFromSSRC(
-                    true, voice_sender_info.ssrc()),
-                timestamp_us));
-        SetOutboundRTPStreamStatsFromVoiceSenderInfo(
-            voice_sender_info, outbound_audio.get());
-        outbound_audio->transport_id = transport_id;
-        report->AddStats(std::move(outbound_audio));
+      report->AddStats(std::move(inbound_audio));
+    }
+    // Outbound
+    for (const cricket::VoiceSenderInfo& voice_sender_info :
+         media_info.voice->senders) {
+      // TODO(nisse): SSRC == 0 currently means none. Delete check when that
+      // is fixed.
+      if (voice_sender_info.ssrc() == 0)
+        continue;
+      std::unique_ptr<RTCOutboundRTPStreamStats> outbound_audio(
+          new RTCOutboundRTPStreamStats(
+              RTCOutboundRTPStreamStatsIDFromSSRC(
+                  true, voice_sender_info.ssrc()),
+              timestamp_us));
+      SetOutboundRTPStreamStatsFromVoiceSenderInfo(
+          voice_sender_info, outbound_audio.get());
+      outbound_audio->transport_id = transport_id;
+      if (voice_sender_info.codec_payload_type) {
+        outbound_audio->codec_id =
+            RTCCodecStatsIDFromDirectionTypeAndPayload(
+                false, true, *voice_sender_info.codec_payload_type);
       }
+      report->AddStats(std::move(outbound_audio));
     }
   }
   // Video
-  if (pc_->session()->video_channel()) {
-    cricket::VideoMediaInfo video_media_info;
-    if (pc_->session()->video_channel()->GetStats(&video_media_info)) {
-      std::string transport_id = RTCTransportStatsIDFromBaseChannel(
-          session_stats.proxy_to_transport, *pc_->session()->video_channel());
-      RTC_DCHECK(!transport_id.empty());
-      // Inbound
-      for (const cricket::VideoReceiverInfo& video_receiver_info :
-           video_media_info.receivers) {
-        // TODO(nisse): SSRC == 0 currently means none. Delete check when that
-        // is fixed.
-        if (video_receiver_info.ssrc() == 0)
-          continue;
-        std::unique_ptr<RTCInboundRTPStreamStats> inbound_video(
-            new RTCInboundRTPStreamStats(
-                RTCInboundRTPStreamStatsIDFromSSRC(
-                    false, video_receiver_info.ssrc()),
-                timestamp_us));
-        SetInboundRTPStreamStatsFromVideoReceiverInfo(
-            video_receiver_info, inbound_video.get());
-        inbound_video->transport_id = transport_id;
-        report->AddStats(std::move(inbound_video));
+  if (media_info.video) {
+    std::string transport_id = RTCTransportStatsIDFromBaseChannel(
+        session_stats.proxy_to_transport, *pc_->session()->video_channel());
+    RTC_DCHECK(!transport_id.empty());
+    // Inbound
+    for (const cricket::VideoReceiverInfo& video_receiver_info :
+         media_info.video->receivers) {
+      // TODO(nisse): SSRC == 0 currently means none. Delete check when that
+      // is fixed.
+      if (video_receiver_info.ssrc() == 0)
+        continue;
+      std::unique_ptr<RTCInboundRTPStreamStats> inbound_video(
+          new RTCInboundRTPStreamStats(
+              RTCInboundRTPStreamStatsIDFromSSRC(
+                  false, video_receiver_info.ssrc()),
+              timestamp_us));
+      SetInboundRTPStreamStatsFromVideoReceiverInfo(
+          video_receiver_info, inbound_video.get());
+      inbound_video->transport_id = transport_id;
+      if (video_receiver_info.codec_payload_type) {
+        inbound_video->codec_id =
+            RTCCodecStatsIDFromDirectionTypeAndPayload(
+                true, false, *video_receiver_info.codec_payload_type);

[hta-webrtc, 2016/11/23 07:37:35]

nit: this shows that the arguments to the call becomes unreadable at the call
site. If the two first arguments had been enums rather than bools, this would
have been more readable.

If no enum is readily available, just keep it as it is.

[hbos, 2016/11/23 09:40:59]

No enum is readily available, keeping as-is.

       }
-      // Outbound
-      for (const cricket::VideoSenderInfo& video_sender_info :
-           video_media_info.senders) {
-        // TODO(nisse): SSRC == 0 currently means none. Delete check when that
-        // is fixed.
-        if (video_sender_info.ssrc() == 0)
-          continue;
-        std::unique_ptr<RTCOutboundRTPStreamStats> outbound_video(
-            new RTCOutboundRTPStreamStats(
-                RTCOutboundRTPStreamStatsIDFromSSRC(
-                    false, video_sender_info.ssrc()),
-                timestamp_us));
-        SetOutboundRTPStreamStatsFromVideoSenderInfo(
-            video_sender_info, outbound_video.get());
-        outbound_video->transport_id = transport_id;
-        report->AddStats(std::move(outbound_video));
+      report->AddStats(std::move(inbound_video));
+    }
+    // Outbound
+    for (const cricket::VideoSenderInfo& video_sender_info :
+         media_info.video->senders) {
+      // TODO(nisse): SSRC == 0 currently means none. Delete check when that
+      // is fixed.
+      if (video_sender_info.ssrc() == 0)
+        continue;
+      std::unique_ptr<RTCOutboundRTPStreamStats> outbound_video(
+          new RTCOutboundRTPStreamStats(
+              RTCOutboundRTPStreamStatsIDFromSSRC(
+                  false, video_sender_info.ssrc()),
+              timestamp_us));
+      SetOutboundRTPStreamStatsFromVideoSenderInfo(
+          video_sender_info, outbound_video.get());
+      outbound_video->transport_id = transport_id;
+      if (video_sender_info.codec_payload_type) {
+        outbound_video->codec_id =
+            RTCCodecStatsIDFromDirectionTypeAndPayload(
+                false, false, *video_sender_info.codec_payload_type);
       }
+      report->AddStats(std::move(outbound_video));
     }
   }
 }
@@ -750,7 +824,7 @@ void RTCStatsCollector::ProduceTransportStats_s(
 }
 
 std::map<std::string, RTCStatsCollector::CertificateStatsPair>
-RTCStatsCollector::PrepareTransportCertificateStats_s(
+RTCStatsCollector::PrepareTransportCertificateStats(
     const SessionStats& session_stats) const {
   RTC_DCHECK(signaling_thread_->IsCurrent());
   std::map<std::string, CertificateStatsPair> transport_cert_stats;
@@ -775,6 +849,26 @@ RTCStatsCollector::PrepareTransportCertificateStats_s(
   return transport_cert_stats;
 }
 
+RTCStatsCollector::MediaInfo RTCStatsCollector::PrepareMediaInfo(
+    const SessionStats& session_stats) const {
+  MediaInfo media_info;
+  if (pc_->session()->voice_channel()) {
+    cricket::VoiceMediaInfo voice_media_info;
+    if (pc_->session()->voice_channel()->GetStats(&voice_media_info)) {
+      media_info.voice = rtc::Optional<cricket::VoiceMediaInfo>(
+          std::move(voice_media_info));
+    }
+  }
+  if (pc_->session()->video_channel()) {
+    cricket::VideoMediaInfo video_media_info;
+    if (pc_->session()->video_channel()->GetStats(&video_media_info)) {
+      media_info.video = rtc::Optional<cricket::VideoMediaInfo>(
+          std::move(video_media_info));
+    }
+  }
+  return media_info;
+}
+
 void RTCStatsCollector::OnDataChannelCreated(DataChannel* channel) {
   channel->SignalOpened.connect(this, &RTCStatsCollector::OnDataChannelOpened);
   channel->SignalClosed.connect(this, &RTCStatsCollector::OnDataChannelClosed);