From 6b4a564d21cac99edfa4ab1cb95d525d40855141 Mon Sep 17 00:00:00 2001 From: "henrik.lundin" Date: Mon, 22 Jun 2015 06:35:09 -0700 Subject: [PATCH] Add UMA logging for target audio bitrate This CL logs the target audio bitrate to a UMA histogram called WebRTC.Audio.TargetBitrateInKbps. It logs the rate when a codec is created, and when the target is explicitly updated. Note that since each codec implementation is free to change or ignore the target value, there is no guarantee that the logged value will actually be used as the target. BUG=chromium:488124 Review URL: https://codereview.webrtc.org/1178053002 Cr-Commit-Position: refs/heads/master@{#9484} --- webrtc/modules/audio_coding/main/acm2/acm_common_defs.h | 3 +++ .../audio_coding/main/acm2/audio_coding_module_impl.cc | 5 ++++- .../main/acm2/audio_coding_module_unittest_oldapi.cc | 4 ++++ webrtc/modules/audio_coding/main/acm2/codec_manager.cc | 5 +++++ webrtc/modules/audio_coding/main/acm2/codec_owner.cc | 6 ++++++ .../modules/audio_coding/main/acm2/codec_owner_unittest.cc | 2 ++ 6 files changed, 24 insertions(+), 1 deletion(-) diff --git a/webrtc/modules/audio_coding/main/acm2/acm_common_defs.h b/webrtc/modules/audio_coding/main/acm2/acm_common_defs.h index 85a287e126..bee10caa7d 100644 --- a/webrtc/modules/audio_coding/main/acm2/acm_common_defs.h +++ b/webrtc/modules/audio_coding/main/acm2/acm_common_defs.h @@ -76,6 +76,9 @@ struct WebRtcACMAudioBuff { uint32_t last_in_timestamp; }; +#define HISTOGRAM_NAME_AUDIO_TARGET_BITRATE_IN_KBPS \ + "WebRTC.Audio.TargetBitrateInKbps" + } // namespace webrtc #endif // WEBRTC_MODULES_AUDIO_CODING_MAIN_ACM2_ACM_COMMON_DEFS_H_ diff --git a/webrtc/modules/audio_coding/main/acm2/audio_coding_module_impl.cc b/webrtc/modules/audio_coding/main/acm2/audio_coding_module_impl.cc index 5536ea4264..c01466f8c0 100644 --- a/webrtc/modules/audio_coding/main/acm2/audio_coding_module_impl.cc +++ b/webrtc/modules/audio_coding/main/acm2/audio_coding_module_impl.cc @@ -23,6 +23,7 @@ #include "webrtc/modules/audio_coding/main/acm2/call_statistics.h" #include "webrtc/system_wrappers/interface/critical_section_wrapper.h" #include "webrtc/system_wrappers/interface/logging.h" +#include "webrtc/system_wrappers/interface/metrics.h" #include "webrtc/system_wrappers/interface/rw_lock_wrapper.h" #include "webrtc/system_wrappers/interface/trace.h" #include "webrtc/typedefs.h" @@ -293,9 +294,11 @@ int AudioCodingModuleImpl::SendBitrate() const { void AudioCodingModuleImpl::SetBitRate(int bitrate_bps) { CriticalSectionScoped lock(acm_crit_sect_); - if (codec_manager_.CurrentEncoder()) { codec_manager_.CurrentEncoder()->SetTargetBitrate(bitrate_bps); + RTC_HISTOGRAM_COUNTS_100( + HISTOGRAM_NAME_AUDIO_TARGET_BITRATE_IN_KBPS, + codec_manager_.CurrentEncoder()->GetTargetBitrate() / 1000); } } diff --git a/webrtc/modules/audio_coding/main/acm2/audio_coding_module_unittest_oldapi.cc b/webrtc/modules/audio_coding/main/acm2/audio_coding_module_unittest_oldapi.cc index 183d526ffc..568ae1ebe7 100644 --- a/webrtc/modules/audio_coding/main/acm2/audio_coding_module_unittest_oldapi.cc +++ b/webrtc/modules/audio_coding/main/acm2/audio_coding_module_unittest_oldapi.cc @@ -1564,6 +1564,10 @@ TEST_F(AcmSenderBitExactnessOldApi, External_Pcmu_20ms) { .Times(AtLeast(1)) .WillRepeatedly( Invoke(&encoder, &AudioEncoderMutablePcmU::EncodeInternal)); + EXPECT_CALL(mock_encoder, GetTargetBitrate()) + .Times(AtLeast(1)) + .WillRepeatedly(Invoke( + &encoder, &AudioEncoderMutablePcmU::GetTargetBitrate)); ASSERT_NO_FATAL_FAILURE( SetUpTestExternalEncoder(&mock_encoder, codec_inst.pltype)); Run("81a9d4c0bb72e9becc43aef124c981e9", "8f9b8750bd80fe26b6cbf6659b89f0f9", diff --git a/webrtc/modules/audio_coding/main/acm2/codec_manager.cc b/webrtc/modules/audio_coding/main/acm2/codec_manager.cc index cad6ee9089..d28dcc6ff5 100644 --- a/webrtc/modules/audio_coding/main/acm2/codec_manager.cc +++ b/webrtc/modules/audio_coding/main/acm2/codec_manager.cc @@ -13,6 +13,8 @@ #include "webrtc/base/checks.h" #include "webrtc/engine_configurations.h" #include "webrtc/modules/audio_coding/main/acm2/acm_codec_database.h" +#include "webrtc/modules/audio_coding/main/acm2/acm_common_defs.h" +#include "webrtc/system_wrappers/interface/metrics.h" #include "webrtc/system_wrappers/interface/trace.h" namespace webrtc { @@ -312,6 +314,9 @@ int CodecManager::RegisterEncoder(const CodecInst& send_codec) { // Check if a change in Rate is required. if (send_codec.rate != send_codec_inst_.rate) { codec_owner_.SpeechEncoder()->SetTargetBitrate(send_codec.rate); + RTC_HISTOGRAM_COUNTS_100( + HISTOGRAM_NAME_AUDIO_TARGET_BITRATE_IN_KBPS, + codec_owner_.SpeechEncoder()->GetTargetBitrate() / 1000); send_codec_inst_.rate = send_codec.rate; } diff --git a/webrtc/modules/audio_coding/main/acm2/codec_owner.cc b/webrtc/modules/audio_coding/main/acm2/codec_owner.cc index 4d214be242..5c1e37b5b0 100644 --- a/webrtc/modules/audio_coding/main/acm2/codec_owner.cc +++ b/webrtc/modules/audio_coding/main/acm2/codec_owner.cc @@ -21,6 +21,8 @@ #include "webrtc/modules/audio_coding/codecs/opus/interface/audio_encoder_opus.h" #include "webrtc/modules/audio_coding/codecs/pcm16b/include/audio_encoder_pcm16b.h" #include "webrtc/modules/audio_coding/codecs/red/audio_encoder_copy_red.h" +#include "webrtc/modules/audio_coding/main/acm2/acm_common_defs.h" +#include "webrtc/system_wrappers/interface/metrics.h" namespace webrtc { namespace acm2 { @@ -180,6 +182,8 @@ void CodecOwner::SetEncoders(const CodecInst& speech_inst, &isac_is_encoder_); external_speech_encoder_ = nullptr; ChangeCngAndRed(cng_payload_type, vad_mode, red_payload_type); + RTC_HISTOGRAM_COUNTS_100(HISTOGRAM_NAME_AUDIO_TARGET_BITRATE_IN_KBPS, + SpeechEncoder()->GetTargetBitrate() / 1000); } void CodecOwner::SetEncoders(AudioEncoderMutable* external_speech_encoder, @@ -190,6 +194,8 @@ void CodecOwner::SetEncoders(AudioEncoderMutable* external_speech_encoder, speech_encoder_.reset(); isac_is_encoder_ = false; ChangeCngAndRed(cng_payload_type, vad_mode, red_payload_type); + RTC_HISTOGRAM_COUNTS_100(HISTOGRAM_NAME_AUDIO_TARGET_BITRATE_IN_KBPS, + SpeechEncoder()->GetTargetBitrate() / 1000); } void CodecOwner::ChangeCngAndRed(int cng_payload_type, diff --git a/webrtc/modules/audio_coding/main/acm2/codec_owner_unittest.cc b/webrtc/modules/audio_coding/main/acm2/codec_owner_unittest.cc index a1366a9b88..b9d4cdd7d1 100644 --- a/webrtc/modules/audio_coding/main/acm2/codec_owner_unittest.cc +++ b/webrtc/modules/audio_coding/main/acm2/codec_owner_unittest.cc @@ -100,6 +100,8 @@ TEST_F(CodecOwnerTest, VerifyCngFrames) { TEST_F(CodecOwnerTest, ExternalEncoder) { MockAudioEncoderMutable external_encoder; + EXPECT_CALL(external_encoder, GetTargetBitrate()) + .WillRepeatedly(Return(-1)); // Flag adaptive bitrate (doesn't matter). codec_owner_.SetEncoders(&external_encoder, -1, VADNormal, -1); const int kSampleRateHz = 8000; const int kPacketSizeSamples = kSampleRateHz / 100;