Implement AudioReceiveStream::GetStats().

webrtc/audio/audio_receive_stream.cc

Index: webrtc/audio/audio_receive_stream.cc
diff --git a/webrtc/audio/audio_receive_stream.cc b/webrtc/audio/audio_receive_stream.cc
index c725e37477af5f36b6a9a18c3556475d12b94b3e..e8fa88bef88990f1e9d81676fcad07a0c99e3ca7 100644
--- a/webrtc/audio/audio_receive_stream.cc
+++ b/webrtc/audio/audio_receive_stream.cc
@@ -16,6 +16,31 @@
 #include "webrtc/base/logging.h"
 #include "webrtc/modules/remote_bitrate_estimator/include/remote_bitrate_estimator.h"
 #include "webrtc/system_wrappers/interface/tick_util.h"
+#include "webrtc/voice_engine/include/voe_base.h"
+#include "webrtc/voice_engine/include/voe_codec.h"
+#include "webrtc/voice_engine/include/voe_neteq_stats.h"
+#include "webrtc/voice_engine/include/voe_rtp_rtcp.h"
+#include "webrtc/voice_engine/include/voe_video_sync.h"
+#include "webrtc/voice_engine/include/voe_volume_control.h"
+
+namespace {
+template<class T> class scoped_voe_interface {

[tommi, 2015/10/19 12:36:24]

nit: ScopedVoEInterface
(I know you're following the scoped_foo convention, but there's also another
ScopedFoo convention which is more aligned with the style guide - but I'll leave
it up to you)

[the sun, 2015/10/19 14:25:02]

Done.

+ public:
+  explicit scoped_voe_interface(webrtc::VoiceEngine* e) : ptr(nullptr) {
+    RTC_DCHECK(e);
+    ptr = T::GetInterface(e);

[tommi, 2015/10/19 12:36:24]

call T::GetInterface(e) in the initializer list

[the sun, 2015/10/19 14:25:02]

Done.

+  }
+  ~scoped_voe_interface() {
+    if (ptr) ptr->Release();

[tommi, 2015/10/19 12:36:24]

nit: two lines

[the sun, 2015/10/19 14:25:02]

Done.

+  }
+  T* operator->() {
+    RTC_DCHECK(ptr);
+    return ptr;
+  }
+ private:
+  T* ptr;

[tommi, 2015/10/19 12:36:24]

ptr_

[the sun, 2015/10/19 14:25:02]

Done.

+};
+} // namespace {
 
 namespace webrtc {
 std::string AudioReceiveStream::Config::Rtp::ToString() const {
@@ -45,13 +70,16 @@ std::string AudioReceiveStream::Config::ToString() const {
 namespace internal {
 AudioReceiveStream::AudioReceiveStream(
       RemoteBitrateEstimator* remote_bitrate_estimator,
-      const webrtc::AudioReceiveStream::Config& config)
+      const webrtc::AudioReceiveStream::Config& config,
+      VoiceEngine* voice_engine)
     : remote_bitrate_estimator_(remote_bitrate_estimator),
       config_(config),
+      voice_engine_(voice_engine),
       rtp_header_parser_(RtpHeaderParser::Create()) {
   LOG(LS_INFO) << "AudioReceiveStream: " << config_.ToString();
   RTC_DCHECK(config.voe_channel_id != -1);
   RTC_DCHECK(remote_bitrate_estimator_ != nullptr);
+  RTC_DCHECK(voice_engine_ != nullptr);
   RTC_DCHECK(rtp_header_parser_ != nullptr);
   for (const auto& ext : config.rtp.extensions) {
     // One-byte-extension local identifiers are in the range 1-14 inclusive.
@@ -77,7 +105,79 @@ AudioReceiveStream::~AudioReceiveStream() {
 }
 
 webrtc::AudioReceiveStream::Stats AudioReceiveStream::GetStats() const {

[tommi, 2015/10/19 12:36:24]

Thread check?

[the sun, 2015/10/19 14:25:02]

Done.

-  return webrtc::AudioReceiveStream::Stats();
+  webrtc::AudioReceiveStream::Stats stats;
+  stats.remote_ssrc = config_.rtp.remote_ssrc;
+  scoped_voe_interface<VoECodec> codec(voice_engine_);
+  scoped_voe_interface<VoENetEqStats> neteq(voice_engine_);
+  scoped_voe_interface<VoERTP_RTCP> rtp(voice_engine_);
+  scoped_voe_interface<VoEVideoSync> sync(voice_engine_);
+  scoped_voe_interface<VoEVolumeControl> volume(voice_engine_);
+  unsigned int ssrc = 0;

[tommi, 2015/10/19 12:36:24]

do we always use unsigned int for ssrc?  do you know why?  (why isn't it just
|int| or a typedef int SSrc?)

[the sun, 2015/10/19 14:25:02]

We're pretty consistent with uint32_t for ssrcs. They're defined here:
https://www.ietf.org/rfc/rfc3550 (page 9, search for "Synchronization source
(SSRC)") as a generic 32-bit number, so basically any int >= 32 bits can be
used; but since they're not used for computations, an unsigned is more suitable;
all we want to do is capture the bit pattern.

A type safe SSRC type would be preferable.

Since you were about to ask, unsigned int (and not uint32_t) is used here
because that is what VoERTP_RTCP::GetRemoteSSRC() uses.

+  webrtc::CallStatistics cs = {0};
+  webrtc::CodecInst ci = {0};
+  // Only collect stats if we have seen some traffic with the SSRC.
+  if (rtp->GetRemoteSSRC(config_.voe_channel_id, ssrc) != -1 &&
+      rtp->GetRTCPStatistics(config_.voe_channel_id, cs) != -1 &&
+      codec->GetRecCodec(config_.voe_channel_id, ci) != -1) {

[tommi, 2015/10/19 12:36:24]

nit: could we convert this to an early return instead of a new scope?

[the sun, 2015/10/19 14:25:02]

Done.

+    stats.bytes_rcvd = cs.bytesReceived;
+    stats.packets_rcvd = cs.packetsReceived;
+    stats.packets_lost = cs.cumulativeLost;
+    stats.fraction_lost = static_cast<float>(cs.fractionLost) / (1 << 8);
+    if (ci.pltype != -1) {

[tommi, 2015/10/19 12:36:24]

no {} (and below for single line if()s)

[the sun, 2015/10/19 14:25:02]

please, point me to the style guide if I've missed that part.

[tommi, 2015/10/19 14:55:44]

Consistency. All I did was go up a few lines to see what was previously done in
this file. See line 64.
I'm also more used to reviewers feeling strongly about this in Chromium.

[the sun, 2015/10/20 08:31:29]

Oops, touché. Fixed that. Blaming sloppy copy+paste tactics.

+      stats.codec_name = ci.plname;
+    }
+
+    stats.ext_seqnum = cs.extendedMax;
+    if (ci.plfreq / 1000 > 0) {
+      stats.jitter_ms = cs.jitterSamples / (ci.plfreq / 1000);
+    }
+    {
+      int jitter_buffer_delay_ms = 0;
+      int playout_buffer_delay_ms = 0;
+      sync->GetDelayEstimate(config_.voe_channel_id, &jitter_buffer_delay_ms,
+                             &playout_buffer_delay_ms);
+      stats.delay_estimate_ms =
+          jitter_buffer_delay_ms + playout_buffer_delay_ms;
+    }
+    {
+      unsigned int level = 0;

[tommi, 2015/10/19 12:36:24]

in some places we use uint32_t for this, but apparently unsigned int in
voe_volume_control.h, maybe it's a legacy thing

[the sun, 2015/10/19 14:25:02]

Yes, I consistently use the types specified in the VoE interfaces when calling
them.

+      if (volume->GetSpeechOutputLevelFullRange(config_.voe_channel_id, level)
+          != -1) {
+        stats.audio_level = level;

[tommi, 2015/10/19 12:36:24]

I wonder which type audio_level is

[the sun, 2015/10/19 14:25:02]

Done.

+      };

[tommi, 2015/10/19 12:36:24]

no ;

[the sun, 2015/10/19 14:25:02]

thx

+    }
+
+    webrtc::NetworkStatistics ns = {0};
+    if (neteq->GetNetworkStatistics(config_.voe_channel_id, ns) != -1) {
+      // Get jitter buffer and total delay (alg + jitter + playout) stats.
+      stats.jitter_buffer_ms = ns.currentBufferSize;
+      stats.jitter_buffer_preferred_ms = ns.preferredBufferSize;
+      stats.expand_rate =
+          static_cast<float>(ns.currentExpandRate) / (1 << 14);

[tommi, 2015/10/19 12:36:24]

can you add a note why we divide all the rates by 1<<14? (or maybe just point me
to the info in case it's obvious)

[the sun, 2015/10/19 14:25:02]

Sure:
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/webrtc...

+      stats.speech_expand_rate =
+          static_cast<float>(ns.currentSpeechExpandRate) / (1 << 14);
+      stats.secondary_decoded_rate =
+          static_cast<float>(ns.currentSecondaryDecodedRate) / (1 << 14);
+      stats.accelerate_rate =
+          static_cast<float>(ns.currentAccelerateRate) / (1 << 14);
+      stats.preemptive_expand_rate =
+          static_cast<float>(ns.currentPreemptiveRate) / (1 << 14);
+    }
+
+    webrtc::AudioDecodingCallStats ds;
+    if (neteq->GetDecodingCallStatistics(config_.voe_channel_id, &ds) != -1) {
+      stats.decoding_calls_to_silence_generator =
+          ds.calls_to_silence_generator;
+      stats.decoding_calls_to_neteq = ds.calls_to_neteq;
+      stats.decoding_normal = ds.decoded_normal;
+      stats.decoding_plc = ds.decoded_plc;
+      stats.decoding_cng = ds.decoded_cng;
+      stats.decoding_plc_cng = ds.decoded_plc_cng;
+    }
+
+    stats.capture_start_ntp_time_ms = cs.capture_start_ntp_time_ms_;
+  }
+  return stats;
 }
 
 const webrtc::AudioReceiveStream::Config& AudioReceiveStream::config() const {