From 0d0b9121282c3f63aaaa242779122f5b0c5db96c Mon Sep 17 00:00:00 2001 From: ivoc Date: Fri, 8 Sep 2017 13:24:21 -0700 Subject: [PATCH] Add and modify a few ANA stats. This CL adds seperate counters for ANA frame length increases and decreases, which gives more insight into what actions are taken. In addition, a new stat is added to track the sum of the uplink packet loss fraction that is set by the ANA FEC controller. BUG=webrtc:8127 Review-Url: https://codereview.webrtc.org/3007243002 Cr-Commit-Position: refs/heads/master@{#19756} --- webrtc/api/audio_codecs/audio_encoder.h | 15 +++++++--- webrtc/api/statstypes.cc | 8 +++-- webrtc/api/statstypes.h | 4 ++- .../engine/webrtcvoiceengine_unittest.cc | 14 +++++++-- .../audio_network_adaptor_impl_unittest.cc | 8 +++-- webrtc/pc/statscollector.cc | 14 +++++++-- webrtc/pc/statscollector_unittest.cc | 30 +++++++++++++++---- 7 files changed, 72 insertions(+), 21 deletions(-) diff --git a/webrtc/api/audio_codecs/audio_encoder.h b/webrtc/api/audio_codecs/audio_encoder.h index a77894b7da..a1f36ae83e 100644 --- a/webrtc/api/audio_codecs/audio_encoder.h +++ b/webrtc/api/audio_codecs/audio_encoder.h @@ -48,10 +48,17 @@ struct ANAStats { // call. If this value is not set, it indicates that the FEC controller is // disabled. rtc::Optional fec_action_counter; - // Number of actions taken by the ANA frame length controller since the start - // of the call. If this value is not set, it indicates that the frame length - // controller is disabled. - rtc::Optional frame_length_action_counter; + // Number of times the ANA frame length controller decided to increase the + // frame length since the start of the call. If this value is not set, it + // indicates that the frame length controller is disabled. + rtc::Optional frame_length_increase_counter; + // Number of times the ANA frame length controller decided to decrease the + // frame length since the start of the call. If this value is not set, it + // indicates that the frame length controller is disabled. + rtc::Optional frame_length_decrease_counter; + // The uplink packet loss fractions as set by the ANA FEC controller. If this + // value is not set, it indicates that the ANA FEC controller is not active. + rtc::Optional uplink_packet_loss_fraction; }; // This is the interface class for encoders in AudioCoding module. Each codec diff --git a/webrtc/api/statstypes.cc b/webrtc/api/statstypes.cc index 67bf49e336..687b6b2fea 100644 --- a/webrtc/api/statstypes.cc +++ b/webrtc/api/statstypes.cc @@ -594,8 +594,12 @@ const char* StatsReport::Value::display_name() const { return "googAnaDtxActionCounter"; case kStatsValueNameAnaFecActionCounter: return "googAnaFecActionCounter"; - case kStatsValueNameAnaFrameLengthActionCounter: - return "googAnaFrameLengthActionCounter"; + case kStatsValueNameAnaFrameLengthIncreaseCounter: + return "googAnaFrameLengthIncreaseCounter"; + case kStatsValueNameAnaFrameLengthDecreaseCounter: + return "googAnaFrameLengthDecreaseCounter"; + case kStatsValueNameAnaUplinkPacketLossFraction: + return "googAnaUplinkPacketLossFraction"; case kStatsValueNameRetransmitBitrate: return "googRetransmitBitrate"; case kStatsValueNameRtt: diff --git a/webrtc/api/statstypes.h b/webrtc/api/statstypes.h index 219995ed83..378b014b17 100644 --- a/webrtc/api/statstypes.h +++ b/webrtc/api/statstypes.h @@ -212,7 +212,9 @@ class StatsReport { kStatsValueNameAnaChannelActionCounter, kStatsValueNameAnaDtxActionCounter, kStatsValueNameAnaFecActionCounter, - kStatsValueNameAnaFrameLengthActionCounter, + kStatsValueNameAnaFrameLengthIncreaseCounter, + kStatsValueNameAnaFrameLengthDecreaseCounter, + kStatsValueNameAnaUplinkPacketLossFraction, kStatsValueNameRetransmitBitrate, kStatsValueNameRtt, kStatsValueNameSecondaryDecodedRate, diff --git a/webrtc/media/engine/webrtcvoiceengine_unittest.cc b/webrtc/media/engine/webrtcvoiceengine_unittest.cc index 67fde12194..edb861ec2e 100644 --- a/webrtc/media/engine/webrtcvoiceengine_unittest.cc +++ b/webrtc/media/engine/webrtcvoiceengine_unittest.cc @@ -550,8 +550,12 @@ class WebRtcVoiceEngineTestFake : public testing::Test { stats.ana_statistics.channel_action_counter = rtc::Optional(432); stats.ana_statistics.dtx_action_counter = rtc::Optional(543); stats.ana_statistics.fec_action_counter = rtc::Optional(654); - stats.ana_statistics.frame_length_action_counter = + stats.ana_statistics.frame_length_increase_counter = rtc::Optional(765); + stats.ana_statistics.frame_length_decrease_counter = + rtc::Optional(876); + stats.ana_statistics.uplink_packet_loss_fraction = + rtc::Optional(987.0); stats.typing_noise_detected = true; return stats; } @@ -591,8 +595,12 @@ class WebRtcVoiceEngineTestFake : public testing::Test { stats.ana_statistics.dtx_action_counter); EXPECT_EQ(info.ana_statistics.fec_action_counter, stats.ana_statistics.fec_action_counter); - EXPECT_EQ(info.ana_statistics.frame_length_action_counter, - stats.ana_statistics.frame_length_action_counter); + EXPECT_EQ(info.ana_statistics.frame_length_increase_counter, + stats.ana_statistics.frame_length_increase_counter); + EXPECT_EQ(info.ana_statistics.frame_length_decrease_counter, + stats.ana_statistics.frame_length_decrease_counter); + EXPECT_EQ(info.ana_statistics.uplink_packet_loss_fraction, + stats.ana_statistics.uplink_packet_loss_fraction); EXPECT_EQ(info.typing_noise_detected, stats.typing_noise_detected && is_sending); } diff --git a/webrtc/modules/audio_coding/audio_network_adaptor/audio_network_adaptor_impl_unittest.cc b/webrtc/modules/audio_coding/audio_network_adaptor/audio_network_adaptor_impl_unittest.cc index c29b1e8c17..5695a38e40 100644 --- a/webrtc/modules/audio_coding/audio_network_adaptor/audio_network_adaptor_impl_unittest.cc +++ b/webrtc/modules/audio_coding/audio_network_adaptor/audio_network_adaptor_impl_unittest.cc @@ -298,8 +298,12 @@ TEST(AudioNetworkAdaptorImplTest, TestANAStats) { default_stats.channel_action_counter); EXPECT_EQ(ana_stats.dtx_action_counter, default_stats.dtx_action_counter); EXPECT_EQ(ana_stats.fec_action_counter, default_stats.fec_action_counter); - EXPECT_EQ(ana_stats.frame_length_action_counter, - default_stats.frame_length_action_counter); + EXPECT_EQ(ana_stats.frame_length_increase_counter, + default_stats.frame_length_increase_counter); + EXPECT_EQ(ana_stats.frame_length_decrease_counter, + default_stats.frame_length_decrease_counter); + EXPECT_EQ(ana_stats.uplink_packet_loss_fraction, + default_stats.uplink_packet_loss_fraction); } } // namespace webrtc diff --git a/webrtc/pc/statscollector.cc b/webrtc/pc/statscollector.cc index b96428353a..a14967a5e3 100644 --- a/webrtc/pc/statscollector.cc +++ b/webrtc/pc/statscollector.cc @@ -240,9 +240,17 @@ void ExtractStats(const cricket::VoiceSenderInfo& info, StatsReport* report) { report->AddInt(StatsReport::kStatsValueNameAnaFecActionCounter, *info.ana_statistics.fec_action_counter); } - if (info.ana_statistics.frame_length_action_counter) { - report->AddInt(StatsReport::kStatsValueNameAnaFrameLengthActionCounter, - *info.ana_statistics.frame_length_action_counter); + if (info.ana_statistics.frame_length_increase_counter) { + report->AddInt(StatsReport::kStatsValueNameAnaFrameLengthIncreaseCounter, + *info.ana_statistics.frame_length_increase_counter); + } + if (info.ana_statistics.frame_length_decrease_counter) { + report->AddInt(StatsReport::kStatsValueNameAnaFrameLengthDecreaseCounter, + *info.ana_statistics.frame_length_decrease_counter); + } + if (info.ana_statistics.uplink_packet_loss_fraction) { + report->AddFloat(StatsReport::kStatsValueNameAnaUplinkPacketLossFraction, + *info.ana_statistics.uplink_packet_loss_fraction); } } diff --git a/webrtc/pc/statscollector_unittest.cc b/webrtc/pc/statscollector_unittest.cc index 57346760e7..6597e95779 100644 --- a/webrtc/pc/statscollector_unittest.cc +++ b/webrtc/pc/statscollector_unittest.cc @@ -454,13 +454,27 @@ void VerifyVoiceSenderInfoReport(const StatsReport* report, ASSERT_TRUE(sinfo.ana_statistics.fec_action_counter); EXPECT_EQ(rtc::ToString(*sinfo.ana_statistics.fec_action_counter), value_in_report); - EXPECT_TRUE(GetValue(report, - StatsReport::kStatsValueNameAnaFrameLengthActionCounter, - &value_in_report)); - ASSERT_TRUE(sinfo.ana_statistics.frame_length_action_counter); + EXPECT_TRUE(GetValue( + report, StatsReport::kStatsValueNameAnaFrameLengthIncreaseCounter, + &value_in_report)); + ASSERT_TRUE(sinfo.ana_statistics.frame_length_increase_counter); EXPECT_EQ(rtc::ToString( - *sinfo.ana_statistics.frame_length_action_counter), + *sinfo.ana_statistics.frame_length_increase_counter), value_in_report); + EXPECT_TRUE(GetValue( + report, StatsReport::kStatsValueNameAnaFrameLengthDecreaseCounter, + &value_in_report)); + ASSERT_TRUE(sinfo.ana_statistics.frame_length_decrease_counter); + EXPECT_EQ(rtc::ToString( + *sinfo.ana_statistics.frame_length_decrease_counter), + value_in_report); + EXPECT_TRUE(GetValue(report, + StatsReport::kStatsValueNameAnaUplinkPacketLossFraction, + &value_in_report)); + ASSERT_TRUE(sinfo.ana_statistics.uplink_packet_loss_fraction); + EXPECT_EQ( + rtc::ToString(*sinfo.ana_statistics.uplink_packet_loss_fraction), + value_in_report); } // Helper methods to avoid duplication of code. @@ -489,8 +503,12 @@ void InitVoiceSenderInfo(cricket::VoiceSenderInfo* voice_sender_info) { rtc::Optional(115); voice_sender_info->ana_statistics.fec_action_counter = rtc::Optional(116); - voice_sender_info->ana_statistics.frame_length_action_counter = + voice_sender_info->ana_statistics.frame_length_increase_counter = rtc::Optional(117); + voice_sender_info->ana_statistics.frame_length_decrease_counter = + rtc::Optional(118); + voice_sender_info->ana_statistics.uplink_packet_loss_fraction = + rtc::Optional(119.0); } void UpdateVoiceSenderInfoFromAudioTrack(