From f4347f7bac66ac9beb255e807929502173865a58 Mon Sep 17 00:00:00 2001 From: Tim Na Date: Wed, 28 Oct 2020 13:51:24 -0700 Subject: [PATCH] VoipStatistics subAPI and implementation. - Adding an interface that fetches lifetime NetEq statistics struct. Bug: webrtc:11989 Change-Id: I871455bccdd53a33dd260f744e03ec81d29fbfd8 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/190200 Commit-Queue: Tim Na Reviewed-by: Karl Wiberg Reviewed-by: Mirko Bonadei Reviewed-by: Ivo Creusen Cr-Commit-Position: refs/heads/master@{#32516} --- api/voip/BUILD.gn | 2 ++ api/voip/voip_engine.h | 5 ++++ api/voip/voip_statistics.h | 34 +++++++++++++++++++++++ audio/voip/audio_channel.cc | 25 +++++++++++++++++ audio/voip/audio_channel.h | 2 ++ audio/voip/audio_ingress.h | 8 ++---- audio/voip/test/audio_channel_unittest.cc | 31 +++++++++++++++++++++ audio/voip/voip_core.cc | 8 ++++++ audio/voip/voip_core.h | 9 +++++- 9 files changed, 117 insertions(+), 7 deletions(-) create mode 100644 api/voip/voip_statistics.h diff --git a/api/voip/BUILD.gn b/api/voip/BUILD.gn index 369a82f3aa..a62dd14207 100644 --- a/api/voip/BUILD.gn +++ b/api/voip/BUILD.gn @@ -16,10 +16,12 @@ rtc_source_set("voip_api") { "voip_dtmf.h", "voip_engine.h", "voip_network.h", + "voip_statistics.h", ] deps = [ "..:array_view", "../audio_codecs:audio_codecs_api", + "../neteq:neteq_api", ] absl_deps = [ "//third_party/abseil-cpp/absl/types:optional" ] } diff --git a/api/voip/voip_engine.h b/api/voip/voip_engine.h index bff261f77a..5724b6b5d9 100644 --- a/api/voip/voip_engine.h +++ b/api/voip/voip_engine.h @@ -17,6 +17,7 @@ class VoipBase; class VoipCodec; class VoipNetwork; class VoipDtmf; +class VoipStatistics; // VoipEngine is the main interface serving as the entry point for all VoIP // APIs. A single instance of VoipEngine should suffice the most of the need for @@ -84,6 +85,10 @@ class VoipEngine { // VoipDtmf provides DTMF event APIs to register and send DTMF events. virtual VoipDtmf& Dtmf() = 0; + + // VoipStatistics provides performance metrics around audio decoding module + // and jitter buffer (NetEq). + virtual VoipStatistics& Statistics() = 0; }; } // namespace webrtc diff --git a/api/voip/voip_statistics.h b/api/voip/voip_statistics.h new file mode 100644 index 0000000000..5f4e174832 --- /dev/null +++ b/api/voip/voip_statistics.h @@ -0,0 +1,34 @@ +/* + * Copyright (c) 2020 The WebRTC project authors. All Rights Reserved. + * + * Use of this source code is governed by a BSD-style license + * that can be found in the LICENSE file in the root of the source + * tree. An additional intellectual property rights grant can be found + * in the file PATENTS. All contributing project authors may + * be found in the AUTHORS file in the root of the source tree. + */ + +#ifndef API_VOIP_VOIP_STATISTICS_H_ +#define API_VOIP_VOIP_STATISTICS_H_ + +#include "api/neteq/neteq.h" +#include "api/voip/voip_base.h" + +namespace webrtc { + +// VoipStatistics interface provides the interfaces for querying metrics around +// the jitter buffer (NetEq) performance. +class VoipStatistics { + public: + // Gets the statistics from NetEq. Returns absl::nullopt when channel_id is + // invalid. + virtual absl::optional GetNetEqStatistics( + ChannelId channel_id) = 0; + + protected: + virtual ~VoipStatistics() = default; +}; + +} // namespace webrtc + +#endif // API_VOIP_VOIP_STATISTICS_H_ diff --git a/audio/voip/audio_channel.cc b/audio/voip/audio_channel.cc index 43d4d0f15d..28bd27020b 100644 --- a/audio/voip/audio_channel.cc +++ b/audio/voip/audio_channel.cc @@ -129,4 +129,29 @@ void AudioChannel::StopPlay() { } } +NetEqLifetimeStatistics AudioChannel::GetNetEqStatistics() { + NetEqLifetimeStatistics neteq_stats; + NetworkStatistics stats = ingress_->GetNetworkStatistics(); + neteq_stats.total_samples_received = stats.totalSamplesReceived; + neteq_stats.concealed_samples = stats.concealedSamples; + neteq_stats.concealment_events = stats.concealmentEvents; + neteq_stats.jitter_buffer_delay_ms = stats.jitterBufferDelayMs; + neteq_stats.jitter_buffer_emitted_count = stats.jitterBufferEmittedCount; + neteq_stats.jitter_buffer_target_delay_ms = stats.jitterBufferTargetDelayMs; + neteq_stats.inserted_samples_for_deceleration = + stats.insertedSamplesForDeceleration; + neteq_stats.removed_samples_for_acceleration = + stats.removedSamplesForAcceleration; + neteq_stats.silent_concealed_samples = stats.silentConcealedSamples; + neteq_stats.fec_packets_received = stats.fecPacketsReceived; + neteq_stats.fec_packets_discarded = stats.fecPacketsDiscarded; + neteq_stats.delayed_packet_outage_samples = stats.delayedPacketOutageSamples; + neteq_stats.relative_packet_arrival_delay_ms = + stats.relativePacketArrivalDelayMs; + neteq_stats.interruption_count = stats.interruptionCount; + neteq_stats.total_interruption_duration_ms = + stats.totalInterruptionDurationMs; + return neteq_stats; +} + } // namespace webrtc diff --git a/audio/voip/audio_channel.h b/audio/voip/audio_channel.h index 04fbfe3a57..9d0e707fae 100644 --- a/audio/voip/audio_channel.h +++ b/audio/voip/audio_channel.h @@ -18,6 +18,7 @@ #include "api/task_queue/task_queue_factory.h" #include "api/voip/voip_base.h" +#include "api/voip/voip_statistics.h" #include "audio/voip/audio_egress.h" #include "audio/voip/audio_ingress.h" #include "modules/rtp_rtcp/source/rtp_rtcp_impl2.h" @@ -81,6 +82,7 @@ class AudioChannel : public rtc::RefCountInterface { void SetReceiveCodecs(const std::map& codecs) { ingress_->SetReceiveCodecs(codecs); } + NetEqLifetimeStatistics GetNetEqStatistics(); private: // ChannelId that this audio channel belongs for logging purpose. diff --git a/audio/voip/audio_ingress.h b/audio/voip/audio_ingress.h index d09de606de..beff6cd6df 100644 --- a/audio/voip/audio_ingress.h +++ b/audio/voip/audio_ingress.h @@ -82,12 +82,8 @@ class AudioIngress : public AudioMixer::Source { NetworkStatistics GetNetworkStatistics() const { NetworkStatistics stats; - acm_receiver_.GetNetworkStatistics(&stats); - return stats; - } - AudioDecodingCallStats GetDecodingStatistics() const { - AudioDecodingCallStats stats; - acm_receiver_.GetDecodingCallStatistics(&stats); + acm_receiver_.GetNetworkStatistics(&stats, + /*get_and_clear_legacy_stats=*/false); return stats; } diff --git a/audio/voip/test/audio_channel_unittest.cc b/audio/voip/test/audio_channel_unittest.cc index ce557823cb..601545bcd8 100644 --- a/audio/voip/test/audio_channel_unittest.cc +++ b/audio/voip/test/audio_channel_unittest.cc @@ -139,5 +139,36 @@ TEST_F(AudioChannelTest, VerifyLocalSsrcAsAssigned) { EXPECT_EQ(rtp.Ssrc(), kLocalSsrc); } +// Check metrics after processing an RTP packet. +TEST_F(AudioChannelTest, TestAudioStatistics) { + rtc::Event event; + auto loop_rtp = [&](const uint8_t* packet, size_t length, Unused) { + audio_channel_->ReceivedRTPPacket( + rtc::ArrayView(packet, length)); + event.Set(); + return true; + }; + EXPECT_CALL(transport_, SendRtp).WillOnce(Invoke(loop_rtp)); + + auto audio_sender = audio_channel_->GetAudioSender(); + audio_sender->SendAudioData(GetAudioFrame(0)); + audio_sender->SendAudioData(GetAudioFrame(1)); + + event.Wait(/*give_up_after_ms=*/1000); + + AudioFrame audio_frame; + audio_mixer_->Mix(/*number_of_channels=*/1, &audio_frame); + + // Check a few fields as we wouldn't have enough samples verify most of them + // here. + absl::optional neteq_stats = + audio_channel_->GetNetEqStatistics(); + EXPECT_TRUE(neteq_stats); + EXPECT_EQ(neteq_stats->total_samples_received, 80ULL); + EXPECT_EQ(neteq_stats->concealed_samples, 0ULL); + EXPECT_EQ(neteq_stats->jitter_buffer_delay_ms, 1600ULL); + EXPECT_EQ(neteq_stats->interruption_count, 0); +} + } // namespace } // namespace webrtc diff --git a/audio/voip/voip_core.cc b/audio/voip/voip_core.cc index 2d752aca58..2c066e1d4a 100644 --- a/audio/voip/voip_core.cc +++ b/audio/voip/voip_core.cc @@ -382,4 +382,12 @@ bool VoipCore::SendDtmfEvent(ChannelId channel, return false; } +absl::optional VoipCore::GetNetEqStatistics( + ChannelId channel) { + if (auto audio_channel = GetChannel(channel)) { + return audio_channel->GetNetEqStatistics(); + } + return absl::nullopt; +} + } // namespace webrtc diff --git a/audio/voip/voip_core.h b/audio/voip/voip_core.h index 11ac6166f0..1993fbef8c 100644 --- a/audio/voip/voip_core.h +++ b/audio/voip/voip_core.h @@ -26,6 +26,7 @@ #include "api/voip/voip_dtmf.h" #include "api/voip/voip_engine.h" #include "api/voip/voip_network.h" +#include "api/voip/voip_statistics.h" #include "audio/audio_transport_impl.h" #include "audio/voip/audio_channel.h" #include "modules/audio_device/include/audio_device.h" @@ -47,7 +48,8 @@ class VoipCore : public VoipEngine, public VoipBase, public VoipNetwork, public VoipCodec, - public VoipDtmf { + public VoipDtmf, + public VoipStatistics { public: ~VoipCore() override = default; @@ -70,6 +72,7 @@ class VoipCore : public VoipEngine, VoipNetwork& Network() override { return *this; } VoipCodec& Codec() override { return *this; } VoipDtmf& Dtmf() override { return *this; } + VoipStatistics& Statistics() override { return *this; } // Implements VoipBase interfaces. absl::optional CreateChannel( @@ -103,6 +106,10 @@ class VoipCore : public VoipEngine, DtmfEvent dtmf_event, int duration_ms) override; + // Implements VoipStatistics interfaces. + absl::optional GetNetEqStatistics( + ChannelId channel) override; + private: // Fetches the corresponding AudioChannel assigned with given |channel|. // Returns nullptr if not found.