Implement AudioReceiveStream::GetStats().

talk/media/webrtc/webrtcvoiceengine_unittest.cc

 /*
  * libjingle
  * Copyright 2008 Google Inc.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions are met:
  *
  *  1. Redistributions of source code must retain the above copyright notice,
  *     this list of conditions and the following disclaimer.
  *  2. Redistributions in binary form must reproduce the above copyright notice,
@@ skipping 20 matching lines @@
 #include "talk/media/base/constants.h"
 #include "talk/media/base/fakemediaengine.h"
 #include "talk/media/base/fakenetworkinterface.h"
 #include "talk/media/base/fakertp.h"
 #include "talk/media/webrtc/fakewebrtccall.h"
 #include "talk/media/webrtc/fakewebrtcvoiceengine.h"
 #include "talk/media/webrtc/webrtcvoiceengine.h"
 #include "webrtc/p2p/base/faketransportcontroller.h"
 #include "talk/session/media/channel.h"
 
-// Tests for the WebRtcVoiceEngine/VoiceChannel code.
-
 using cricket::kRtpAudioLevelHeaderExtension;
 using cricket::kRtpAbsoluteSenderTimeHeaderExtension;
 
-static const cricket::AudioCodec kPcmuCodec(0, "PCMU", 8000, 64000, 1, 0);
-static const cricket::AudioCodec kIsacCodec(103, "ISAC", 16000, 32000, 1, 0);
-static const cricket::AudioCodec kOpusCodec(111, "opus", 48000, 64000, 2, 0);
-static const cricket::AudioCodec kG722CodecVoE(9, "G722", 16000, 64000, 1, 0);
-static const cricket::AudioCodec kG722CodecSdp(9, "G722", 8000, 64000, 1, 0);
-static const cricket::AudioCodec kRedCodec(117, "red", 8000, 0, 1, 0);
-static const cricket::AudioCodec kCn8000Codec(13, "CN", 8000, 0, 1, 0);
-static const cricket::AudioCodec kCn16000Codec(105, "CN", 16000, 0, 1, 0);
-static const cricket::AudioCodec
-    kTelephoneEventCodec(106, "telephone-event", 8000, 0, 1, 0);
-static const cricket::AudioCodec* const kAudioCodecs[] = {
+namespace {
+
+const cricket::AudioCodec kPcmuCodec(0, "PCMU", 8000, 64000, 1, 0);
+const cricket::AudioCodec kIsacCodec(103, "ISAC", 16000, 32000, 1, 0);
+const cricket::AudioCodec kOpusCodec(111, "opus", 48000, 64000, 2, 0);
+const cricket::AudioCodec kG722CodecVoE(9, "G722", 16000, 64000, 1, 0);
+const cricket::AudioCodec kG722CodecSdp(9, "G722", 8000, 64000, 1, 0);
+const cricket::AudioCodec kRedCodec(117, "red", 8000, 0, 1, 0);
+const cricket::AudioCodec kCn8000Codec(13, "CN", 8000, 0, 1, 0);
+const cricket::AudioCodec kCn16000Codec(105, "CN", 16000, 0, 1, 0);
+const cricket::AudioCodec kTelephoneEventCodec(106, "telephone-event", 8000, 0,
+                                               1, 0);
+const cricket::AudioCodec* const kAudioCodecs[] = {
     &kPcmuCodec, &kIsacCodec, &kOpusCodec, &kG722CodecVoE, &kRedCodec,
     &kCn8000Codec, &kCn16000Codec, &kTelephoneEventCodec,
 };
-static uint32_t kSsrc1 = 0x99;
-static uint32_t kSsrc2 = 0x98;
+const uint32_t kSsrc1 = 0x99;
+const uint32_t kSsrc2 = 0x98;
 
 class FakeVoEWrapper : public cricket::VoEWrapper {
  public:
   explicit FakeVoEWrapper(cricket::FakeWebRtcVoiceEngine* engine)
       : cricket::VoEWrapper(engine,  // processing
                             engine,  // base
                             engine,  // codec
                             engine,  // dtmf
                             engine,  // hw
-                            engine,  // neteq
                             engine,  // network
                             engine,  // rtp
-                            engine,  // sync
                             engine) {  // volume
   }
 };
 
 class FakeVoETraceWrapper : public cricket::VoETraceWrapper {
  public:
   int SetTraceFilter(const unsigned int filter) override {
     filter_ = filter;
     return 0;
   }
   int SetTraceFile(const char* fileNameUTF8) override { return 0; }
   int SetTraceCallback(webrtc::TraceCallback* callback) override { return 0; }
   unsigned int filter_;
 };
+} // namespace

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

nit: 2 spaces

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

Done.

 
 class WebRtcVoiceEngineTestFake : public testing::Test {
  public:
   WebRtcVoiceEngineTestFake()
       : call_(webrtc::Call::Config()),
         voe_(kAudioCodecs, ARRAY_SIZE(kAudioCodecs)),
         trace_wrapper_(new FakeVoETraceWrapper()),
         engine_(new FakeVoEWrapper(&voe_), trace_wrapper_),
         channel_(nullptr) {
     send_parameters_.codecs.push_back(kPcmuCodec);
@@ skipping 187 matching lines @@
     EXPECT_NE(channel_num, new_channel_num);
     EXPECT_EQ(id, voe_.GetReceiveRtpExtensionId(new_channel_num, ext));
 
     // Ensure all extensions go back off with an empty list.
     parameters.extensions.clear();
     EXPECT_TRUE(channel_->SetRecvParameters(parameters));
     EXPECT_EQ(-1, voe_.GetReceiveRtpExtensionId(channel_num, ext));
     EXPECT_EQ(-1, voe_.GetReceiveRtpExtensionId(new_channel_num, ext));
   }
 
+  const webrtc::AudioReceiveStream::Stats& GetAudioReceiveStreamStats() const {
+    static webrtc::AudioReceiveStream::Stats stats;
+    if (stats.remote_ssrc == 0) {
+      stats.remote_ssrc = 123;
+      stats.bytes_rcvd = 456;
+      stats.packets_rcvd = 768;
+      stats.packets_lost = 101;
+      stats.fraction_lost = 23.45f;
+      stats.codec_name = "codec_name";
+      stats.ext_seqnum = 678;
+      stats.jitter_ms = 901;
+      stats.jitter_buffer_ms = 234;
+      stats.jitter_buffer_preferred_ms = 567;
+      stats.delay_estimate_ms = 890;
+      stats.audio_level = 1234;
+      stats.expand_rate = 5.67f;
+      stats.speech_expand_rate = 8.90f;
+      stats.secondary_decoded_rate = 1.23f;
+      stats.accelerate_rate = 4.56f;
+      stats.preemptive_expand_rate = 7.89f;
+      stats.decoding_calls_to_silence_generator = 012;
+      stats.decoding_calls_to_neteq = 345;
+      stats.decoding_normal = 67890;
+      stats.decoding_plc = 1234;
+      stats.decoding_cng = 5678;
+      stats.decoding_plc_cng = 9012;
+      stats.capture_start_ntp_time_ms = 3456;
+    }
+    return stats;
+  }
+  void SetAudioReceiveStreamStats() {
+    for (auto s : call_.GetAudioReceiveStreams()) {

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

nit: no curlies.
also, just to make it clear that this is not by-value (i.e. creating a copy each
time), use auto*

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

Yes, curlies.

1. Style guide avoids the issue.
2. Surrounding code (libjingle) generally use curlies for one-liners; style
guide says prefer consistency.
3. Arguments in favor of using curlies (consistency, less risk of copy/paste
bugs, bad formatting doesn't cause reader misinterpretation) trumps arguments
against (an extra line).

:)

auto* is more readable, thanks.

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

Re 2. I thought the opposite.
Re 3. (I'll need to break that down)
  - consistency - I don't think that's in favor or against {} in general
  - less risk of copy/paste bugs - nah where there's a will, there's a way, see
next line.
  - bad formatting doesn't cause reader misinterpretation - you'd think not :) 
see slide 33 here:
https://docs.google.com/presentation/d/1WAa-k6n9v9XEUPRxYODrCH5Bvng6KckGkr-7Z...
(it's not clear where the author went wrong, but for sure it fooled the
reviewer)

I understand your point though and don't want to keep you occupied with bike
shedding.  In the end, you're right since this file uses curly brackets for
single line statements like this.

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

Wonder how often these debates pop up? Goes to show that there should just be a
decision in the style guide...

But since we're on the topic, allow me to shoot back:

- Consistency - yes, if consistently using brackets, there is one less
possibility for the reader to take into account when mentally parsing the code.
- Copy/paste - agree the risk isn't big, but how about badly written macros?
Consistent bracketing avoids such issues.
- As I see it, in the example you give, the problem *is* mixed bracketing. It's
more likely the reviewer (and writer) would have spotted a misaligned } even if
the formatting was messed up.

As a friend of mine once said: "Religions? All good. Just don't try to convert
me." :)

+      s->SetStats(GetAudioReceiveStreamStats());
+    }
+  }
+  void VerifyVoiceReceiverInfo(const cricket::VoiceReceiverInfo& info) {
+    const auto& kStats = GetAudioReceiveStreamStats();
+    EXPECT_EQ(info.local_stats.front().ssrc, kStats.remote_ssrc);
+    EXPECT_EQ(info.bytes_rcvd, kStats.bytes_rcvd);
+    EXPECT_EQ(info.packets_rcvd, kStats.packets_rcvd);
+    EXPECT_EQ(info.packets_lost, kStats.packets_lost);
+    EXPECT_EQ(info.fraction_lost, kStats.fraction_lost);
+    EXPECT_EQ(info.codec_name, kStats.codec_name);
+    EXPECT_EQ(info.ext_seqnum, kStats.ext_seqnum);
+    EXPECT_EQ(info.jitter_ms, kStats.jitter_ms);
+    EXPECT_EQ(info.jitter_buffer_ms, kStats.jitter_buffer_ms);
+    EXPECT_EQ(info.jitter_buffer_preferred_ms,
+              kStats.jitter_buffer_preferred_ms);
+    EXPECT_EQ(info.delay_estimate_ms, kStats.delay_estimate_ms);
+    EXPECT_EQ(info.audio_level, kStats.audio_level);
+    EXPECT_EQ(info.expand_rate, kStats.expand_rate);
+    EXPECT_EQ(info.speech_expand_rate, kStats.speech_expand_rate);
+    EXPECT_EQ(info.secondary_decoded_rate, kStats.secondary_decoded_rate);
+    EXPECT_EQ(info.accelerate_rate, kStats.accelerate_rate);
+    EXPECT_EQ(info.preemptive_expand_rate, kStats.preemptive_expand_rate);
+    EXPECT_EQ(info.decoding_calls_to_silence_generator,
+              kStats.decoding_calls_to_silence_generator);
+    EXPECT_EQ(info.decoding_calls_to_neteq, kStats.decoding_calls_to_neteq);
+    EXPECT_EQ(info.decoding_normal, kStats.decoding_normal);
+    EXPECT_EQ(info.decoding_plc, kStats.decoding_plc);
+    EXPECT_EQ(info.decoding_cng, kStats.decoding_cng);
+    EXPECT_EQ(info.decoding_plc_cng, kStats.decoding_plc_cng);
+    EXPECT_EQ(info.capture_start_ntp_time_ms, kStats.capture_start_ntp_time_ms);
+  }
+
  protected:
   cricket::FakeCall call_;
   cricket::FakeWebRtcVoiceEngine voe_;
   FakeVoETraceWrapper* trace_wrapper_;
   cricket::WebRtcVoiceEngine engine_;
   cricket::VoiceMediaChannel* channel_;
 
   cricket::AudioSendParameters send_parameters_;
   cricket::AudioRecvParameters recv_parameters_;
   cricket::AudioOptions options_adjust_agc_;
@@ skipping 1695 matching lines @@
     EXPECT_EQ(kPcmuCodec.name, info.senders[i].codec_name);
     EXPECT_EQ(cricket::kIntStatValue, info.senders[i].bytes_sent);
     EXPECT_EQ(cricket::kIntStatValue, info.senders[i].packets_sent);
     EXPECT_EQ(cricket::kIntStatValue, info.senders[i].packets_lost);
     EXPECT_EQ(cricket::kFractionLostStatValue, info.senders[i].fraction_lost);
     EXPECT_EQ(cricket::kIntStatValue, info.senders[i].ext_seqnum);
     EXPECT_EQ(cricket::kIntStatValue, info.senders[i].rtt_ms);
     EXPECT_EQ(cricket::kIntStatValue, info.senders[i].jitter_ms);
     EXPECT_EQ(kPcmuCodec.name, info.senders[i].codec_name);
   }
-  EXPECT_EQ(0u, info.receivers.size());
+
+  // We have added one receive stream, but its stats should be empty.
+  EXPECT_EQ(info.receivers.size(), 1u);
+  EXPECT_EQ(info.receivers[0].local_stats.front().ssrc, 0);
 
   // Registered stream's remote SSRC is kSsrc2. Send a packet with SSRC=1.
   // We should drop the packet and no stats should be available.
   DeliverPacket(kPcmuFrame, sizeof(kPcmuFrame));
   EXPECT_EQ(true, channel_->GetStats(&info));
+  EXPECT_EQ(info.receivers.size(), 1u);
+  EXPECT_EQ(info.receivers[0].local_stats.front().ssrc, 0);
+
+  // Remove the kSsrc2 stream.
+  EXPECT_TRUE(channel_->RemoveRecvStream(kSsrc2));
+  EXPECT_EQ(true, channel_->GetStats(&info));
   EXPECT_EQ(0u, info.receivers.size());
 
-  // Remove the kSsrc2 stream and deliver a new packet - a default receive
-  // stream should be created and we should see stats.
-  EXPECT_TRUE(channel_->RemoveRecvStream(kSsrc2));
+  // Deliver a new packet - a default receive stream should be created and we
+  // should see stats.
   DeliverPacket(kPcmuFrame, sizeof(kPcmuFrame));
+  SetAudioReceiveStreamStats();
   EXPECT_EQ(true, channel_->GetStats(&info));
   EXPECT_EQ(1u, info.receivers.size());
-
-  EXPECT_EQ(cricket::kIntStatValue, info.receivers[0].bytes_rcvd);
-  EXPECT_EQ(cricket::kIntStatValue, info.receivers[0].packets_rcvd);
-  EXPECT_EQ(cricket::kIntStatValue, info.receivers[0].packets_lost);
-  EXPECT_EQ(cricket::kIntStatValue, info.receivers[0].ext_seqnum);
-  EXPECT_EQ(kPcmuCodec.name, info.receivers[0].codec_name);
-  EXPECT_EQ(static_cast<float>(cricket::kNetStats.currentExpandRate) /
-      (1 << 14), info.receivers[0].expand_rate);
-  EXPECT_EQ(static_cast<float>(cricket::kNetStats.currentSpeechExpandRate) /
-      (1 << 14), info.receivers[0].speech_expand_rate);
-  EXPECT_EQ(static_cast<float>(cricket::kNetStats.currentSecondaryDecodedRate) /
-      (1 << 14), info.receivers[0].secondary_decoded_rate);
-  EXPECT_EQ(
-      static_cast<float>(cricket::kNetStats.currentAccelerateRate) / (1 << 14),
-      info.receivers[0].accelerate_rate);
-  EXPECT_EQ(
-      static_cast<float>(cricket::kNetStats.currentPreemptiveRate) / (1 << 14),
-      info.receivers[0].preemptive_expand_rate);
+  VerifyVoiceReceiverInfo(info.receivers[0]);
 }
 
 // Test that we can add and remove receive streams, and do proper send/playout.
 // We can receive on multiple streams while sending one stream.
 TEST_F(WebRtcVoiceEngineTestFake, PlayoutWithMultipleStreams) {
   EXPECT_TRUE(SetupEngineWithSendStream());
   int channel_num1 = voe_.GetLastChannel();
 
   // Start playout without a receive stream.
   EXPECT_TRUE(channel_->SetSendParameters(send_parameters_));
@@ skipping 240 matching lines @@
   EXPECT_EQ(cricket::kIntStatValue, info.senders[0].jitter_ms);
   EXPECT_EQ(kPcmuCodec.name, info.senders[0].codec_name);
   // TODO(sriniv): Add testing for more fields. These are not populated
   // in FakeWebrtcVoiceEngine yet.
   // EXPECT_EQ(cricket::kIntStatValue, info.senders[0].audio_level);
   // EXPECT_EQ(cricket::kIntStatValue, info.senders[0].echo_delay_median_ms);
   // EXPECT_EQ(cricket::kIntStatValue, info.senders[0].echo_delay_std_ms);
   // EXPECT_EQ(cricket::kIntStatValue, info.senders[0].echo_return_loss);
   // EXPECT_EQ(cricket::kIntStatValue,
   //           info.senders[0].echo_return_loss_enhancement);
-  EXPECT_EQ(0u, info.receivers.size());
+  // We have added one receive stream, but its stats should be empty.
+  EXPECT_EQ(info.receivers.size(), 1u);
+  EXPECT_EQ(info.receivers[0].local_stats.front().ssrc, 0);
 
   // Registered stream's remote SSRC is kSsrc2. Send a packet with SSRC=1.
   // We should drop the packet and no stats should be available.
   DeliverPacket(kPcmuFrame, sizeof(kPcmuFrame));
   EXPECT_EQ(true, channel_->GetStats(&info));
+  EXPECT_EQ(info.receivers.size(), 1u);
+  EXPECT_EQ(info.receivers[0].local_stats.front().ssrc, 0);
+
+  // Remove the kSsrc2 stream.
+  EXPECT_TRUE(channel_->RemoveRecvStream(kSsrc2));
+  EXPECT_EQ(true, channel_->GetStats(&info));
   EXPECT_EQ(0u, info.receivers.size());
 
-  // Remove the kSsrc2 stream and deliver a new packet - a default receive
-  // stream should be created and we should see stats.
-  EXPECT_TRUE(channel_->RemoveRecvStream(kSsrc2));
+  // Deliver a new packet - a default receive stream should be created and we
+  // should see stats.
   DeliverPacket(kPcmuFrame, sizeof(kPcmuFrame));
+  SetAudioReceiveStreamStats();
   EXPECT_EQ(true, channel_->GetStats(&info));
   EXPECT_EQ(1u, info.receivers.size());
-
-  EXPECT_EQ(cricket::kIntStatValue, info.receivers[0].bytes_rcvd);
-  EXPECT_EQ(cricket::kIntStatValue, info.receivers[0].packets_rcvd);
-  EXPECT_EQ(cricket::kIntStatValue, info.receivers[0].packets_lost);
-  EXPECT_EQ(cricket::kIntStatValue, info.receivers[0].ext_seqnum);
-  EXPECT_EQ(kPcmuCodec.name, info.receivers[0].codec_name);
-  EXPECT_EQ(static_cast<float>(cricket::kNetStats.currentExpandRate) /
-      (1 << 14), info.receivers[0].expand_rate);
-  EXPECT_EQ(static_cast<float>(cricket::kNetStats.currentSpeechExpandRate) /
-      (1 << 14), info.receivers[0].speech_expand_rate);
-  EXPECT_EQ(static_cast<float>(cricket::kNetStats.currentSecondaryDecodedRate) /
-      (1 << 14), info.receivers[0].secondary_decoded_rate);
-  // TODO(sriniv): Add testing for more receiver fields.
+  VerifyVoiceReceiverInfo(info.receivers[0]);
 }
 
 // Test that we can set the outgoing SSRC properly with multiple streams.
 // SSRC is set in SetupEngine by calling AddSendStream.
 TEST_F(WebRtcVoiceEngineTestFake, SetSendSsrcWithMultipleStreams) {
   EXPECT_TRUE(SetupEngineWithSendStream());
   int channel_num1 = voe_.GetLastChannel();
   unsigned int send_ssrc;
   EXPECT_EQ(0, voe_.GetLocalSSRC(channel_num1, send_ssrc));
   EXPECT_EQ(kSsrc1, send_ssrc);
@@ skipping 918 matching lines @@
   cricket::WebRtcVoiceEngine engine;
   EXPECT_TRUE(engine.Init(rtc::Thread::Current()));
   rtc::scoped_ptr<webrtc::Call> call(
       webrtc::Call::Create(webrtc::Call::Config()));
   cricket::WebRtcVoiceMediaChannel channel(&engine, cricket::AudioOptions(),
                                            call.get());
   cricket::AudioRecvParameters parameters;
   parameters.codecs = engine.codecs();
   EXPECT_TRUE(channel.SetRecvParameters(parameters));
 }