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 bee10caa7..85a287e12 100644 --- a/webrtc/modules/audio_coding/main/acm2/acm_common_defs.h +++ b/webrtc/modules/audio_coding/main/acm2/acm_common_defs.h @@ -76,9 +76,6 @@ 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 c01466f8c..9c3183271 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 @@ -122,6 +122,14 @@ void ConvertEncodedInfoToFragmentationHeader( } } // namespace +void AudioCodingModuleImpl::ChangeLogger::MaybeLog(int value) { + if (value != last_value_ || first_time_) { + first_time_ = false; + last_value_ = value; + RTC_HISTOGRAM_COUNTS_100(histogram_name_, value); + } +} + AudioCodingModuleImpl::AudioCodingModuleImpl( const AudioCodingModule::Config& config) : acm_crit_sect_(CriticalSectionWrapper::CreateCriticalSection()), @@ -129,6 +137,7 @@ AudioCodingModuleImpl::AudioCodingModuleImpl( expected_codec_ts_(0xD87F3F9F), expected_in_ts_(0xD87F3F9F), receiver_(config), + bitrate_logger_("WebRTC.Audio.TargetBitrateInKbps"), previous_pltype_(255), aux_rtp_header_(NULL), receiver_initialized_(false), @@ -185,6 +194,7 @@ int32_t AudioCodingModuleImpl::Encode(const InputData& input_data) { encoded_info = audio_encoder->Encode(rtp_timestamp, input_data.audio, input_data.length_per_channel, sizeof(stream), stream); + bitrate_logger_.MaybeLog(audio_encoder->GetTargetBitrate() / 1000); if (encoded_info.encoded_bytes == 0 && !encoded_info.send_even_if_empty) { // Not enough data. return 0; @@ -296,9 +306,6 @@ 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_impl.h b/webrtc/modules/audio_coding/main/acm2/audio_coding_module_impl.h index 6ef80bc66..19ca01bf9 100644 --- a/webrtc/modules/audio_coding/main/acm2/audio_coding_module_impl.h +++ b/webrtc/modules/audio_coding/main/acm2/audio_coding_module_impl.h @@ -250,6 +250,22 @@ class AudioCodingModuleImpl : public AudioCodingModule { int16_t buffer[WEBRTC_10MS_PCM_AUDIO]; }; + // This member class writes values to the named UMA histogram, but only if + // the value has changed since the last time (and always for the first call). + class ChangeLogger { + public: + explicit ChangeLogger(const std::string& histogram_name) + : histogram_name_(histogram_name) {} + // Logs the new value if it is different from the last logged value, or if + // this is the first call. + void MaybeLog(int value); + + private: + int last_value_ = 0; + int first_time_ = true; + const std::string histogram_name_; + }; + int Add10MsDataInternal(const AudioFrame& audio_frame, InputData* input_data) EXCLUSIVE_LOCKS_REQUIRED(acm_crit_sect_); int Encode(const InputData& input_data) @@ -285,6 +301,7 @@ class AudioCodingModuleImpl : public AudioCodingModule { uint32_t expected_in_ts_ GUARDED_BY(acm_crit_sect_); ACMResampler resampler_ GUARDED_BY(acm_crit_sect_); AcmReceiver receiver_; // AcmReceiver has it's own internal lock. + ChangeLogger bitrate_logger_ GUARDED_BY(acm_crit_sect_); CodecManager codec_manager_ GUARDED_BY(acm_crit_sect_); // This is to keep track of CN instances where we can send DTMFs. diff --git a/webrtc/modules/audio_coding/main/acm2/codec_manager.cc b/webrtc/modules/audio_coding/main/acm2/codec_manager.cc index d28dcc6ff..cad6ee908 100644 --- a/webrtc/modules/audio_coding/main/acm2/codec_manager.cc +++ b/webrtc/modules/audio_coding/main/acm2/codec_manager.cc @@ -13,8 +13,6 @@ #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 { @@ -314,9 +312,6 @@ 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 5c1e37b5b..4d214be24 100644 --- a/webrtc/modules/audio_coding/main/acm2/codec_owner.cc +++ b/webrtc/modules/audio_coding/main/acm2/codec_owner.cc @@ -21,8 +21,6 @@ #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 { @@ -182,8 +180,6 @@ 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, @@ -194,8 +190,6 @@ 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 b9d4cdd7d..a1366a9b8 100644 --- a/webrtc/modules/audio_coding/main/acm2/codec_owner_unittest.cc +++ b/webrtc/modules/audio_coding/main/acm2/codec_owner_unittest.cc @@ -100,8 +100,6 @@ 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;