From 79ea89ee742532d747d9380da1b4520782befe10 Mon Sep 17 00:00:00 2001 From: Fredrik Hernqvist Date: Wed, 12 Apr 2023 16:31:53 +0200 Subject: [PATCH] Add histogram for audio mixer maximum source count. This adds the histogram WebRTC.Audio.AudioMixer.NewHighestSourceCount which logs the highest number of sources an AudioMixer has had. The statistic is logged whenever the highest number of sources increases. This allows us to differentiate the statistic to see how many times the mixer has had a certain maximum number of sources. Chromium CL: https://chromium-review.googlesource.com/c/chromium/src/+/4414896 Bug: chromium:1430806 Change-Id: Iab92e201a0b667741cc8f3bbbed92fa989d7fcda Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/300860 Reviewed-by: Olga Sharonova Commit-Queue: Fredrik Hernqvist Reviewed-by: Alessio Bazzica Cr-Commit-Position: refs/heads/main@{#39833} --- modules/audio_mixer/BUILD.gn | 1 + modules/audio_mixer/audio_mixer_impl.cc | 12 ++++++ modules/audio_mixer/audio_mixer_impl.h | 5 +++ .../audio_mixer/audio_mixer_impl_unittest.cc | 38 +++++++++++++++++++ 4 files changed, 56 insertions(+) diff --git a/modules/audio_mixer/BUILD.gn b/modules/audio_mixer/BUILD.gn index fe20f3d6c7..fb038bf677 100644 --- a/modules/audio_mixer/BUILD.gn +++ b/modules/audio_mixer/BUILD.gn @@ -122,6 +122,7 @@ if (rtc_include_tests) { "../../rtc_base:checks", "../../rtc_base:stringutils", "../../rtc_base:task_queue_for_test", + "../../system_wrappers:metrics", "../../test:test_support", ] } diff --git a/modules/audio_mixer/audio_mixer_impl.cc b/modules/audio_mixer/audio_mixer_impl.cc index 0c203a1d9f..666497484b 100644 --- a/modules/audio_mixer/audio_mixer_impl.cc +++ b/modules/audio_mixer/audio_mixer_impl.cc @@ -22,6 +22,7 @@ #include "rtc_base/checks.h" #include "rtc_base/logging.h" #include "rtc_base/trace_event.h" +#include "system_wrappers/include/metrics.h" namespace webrtc { @@ -186,6 +187,7 @@ bool AudioMixerImpl::AddSource(Source* audio_source) { << "Source already added to mixer"; audio_source_list_.emplace_back(new SourceStatus(audio_source, false, 0)); helper_containers_->resize(audio_source_list_.size()); + UpdateSourceCountStats(); return true; } @@ -263,4 +265,14 @@ bool AudioMixerImpl::GetAudioSourceMixabilityStatusForTest( RTC_LOG(LS_ERROR) << "Audio source unknown"; return false; } + +void AudioMixerImpl::UpdateSourceCountStats() { + size_t current_source_count = audio_source_list_.size(); + // Log to the histogram whenever the maximum number of sources increases. + if (current_source_count > max_source_count_ever_) { + RTC_HISTOGRAM_COUNTS_LINEAR("WebRTC.Audio.AudioMixer.NewHighestSourceCount", + current_source_count, 1, 20, 20); + max_source_count_ever_ = current_source_count; + } +} } // namespace webrtc diff --git a/modules/audio_mixer/audio_mixer_impl.h b/modules/audio_mixer/audio_mixer_impl.h index 76b1131777..e1040defd7 100644 --- a/modules/audio_mixer/audio_mixer_impl.h +++ b/modules/audio_mixer/audio_mixer_impl.h @@ -71,6 +71,8 @@ class AudioMixerImpl : public AudioMixer { private: struct HelperContainers; + void UpdateSourceCountStats() RTC_EXCLUSIVE_LOCKS_REQUIRED(mutex_); + // Compute what audio sources to mix from audio_source_list_. Ramp // in and out. Update mixed status. Mixes up to // kMaximumAmountOfMixedAudioSources audio sources. @@ -94,6 +96,9 @@ class AudioMixerImpl : public AudioMixer { // Component that handles actual adding of audio frames. FrameCombiner frame_combiner_; + + // The highest source count this mixer has ever had. Used for UMA stats. + size_t max_source_count_ever_ = 0; }; } // namespace webrtc diff --git a/modules/audio_mixer/audio_mixer_impl_unittest.cc b/modules/audio_mixer/audio_mixer_impl_unittest.cc index e4b12a7000..8022332b27 100644 --- a/modules/audio_mixer/audio_mixer_impl_unittest.cc +++ b/modules/audio_mixer/audio_mixer_impl_unittest.cc @@ -28,6 +28,7 @@ #include "rtc_base/checks.h" #include "rtc_base/strings/string_builder.h" #include "rtc_base/task_queue_for_test.h" +#include "system_wrappers/include/metrics.h" #include "test/gmock.h" #include "test/gtest.h" @@ -42,6 +43,8 @@ namespace webrtc { namespace { constexpr int kDefaultSampleRateHz = 48000; +const char kSourceCountHistogramName[] = + "WebRTC.Audio.AudioMixer.NewHighestSourceCount"; // Utility function that resets the frame member variables with // sensible defaults. @@ -214,6 +217,41 @@ TEST(AudioMixer, LargestEnergyVadActiveMixed) { } } +TEST(AudioMixer, UpdatesSourceCountHistogram) { + constexpr int kAudioSourcesGroup1 = 5; + constexpr int kAudioSourcesGroup2 = 3; + + const auto mixer = AudioMixerImpl::Create(); + + MockMixerAudioSource participants[kAudioSourcesGroup1 + kAudioSourcesGroup2]; + + // Add the sources in group 1. + for (int i = 0; i < kAudioSourcesGroup1; ++i) { + EXPECT_TRUE(mixer->AddSource(&participants[i])); + EXPECT_EQ(i + 1, metrics::NumSamples(kSourceCountHistogramName)); + EXPECT_EQ(1, metrics::NumEvents(kSourceCountHistogramName, i + 1)); + } + // Remove the sources again. + for (int i = 0; i < kAudioSourcesGroup1; ++i) { + mixer->RemoveSource(&participants[i]); + } + // Add the first group again. This should not add anything new to the + // histogram. + for (int i = 0; i < kAudioSourcesGroup1; ++i) { + EXPECT_TRUE(mixer->AddSource(&participants[i])); + EXPECT_EQ(kAudioSourcesGroup1, + metrics::NumSamples(kSourceCountHistogramName)); + EXPECT_EQ(1, metrics::NumEvents(kSourceCountHistogramName, i + 1)); + } + // Add the second group. This adds to the histogram again. + for (int i = kAudioSourcesGroup1; + i < kAudioSourcesGroup1 + kAudioSourcesGroup2; ++i) { + EXPECT_TRUE(mixer->AddSource(&participants[i])); + EXPECT_EQ(i + 1, metrics::NumSamples(kSourceCountHistogramName)); + EXPECT_EQ(1, metrics::NumEvents(kSourceCountHistogramName, i + 1)); + } +} + TEST(AudioMixer, FrameNotModifiedForSingleParticipant) { const auto mixer = AudioMixerImpl::Create();