admin/webrtc_m130
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Improve code quality in modules/audio_processing/agc/

Browse Source 
- Switch from ptr+size to rtc::ArrayView
- Remove `AgcManagerDirect::sample_rate_hz_` since it's always 16 kHz
- Stop passing nullptr in agc_manager_direct_unittest.cc when
  `AgcManagerDirect::Process()` is called
- Allow to correctly run the tests added in the child CL (see [1])

[1] https://webrtc-review.googlesource.com/c/src/+/250141

Bug: webrtc:7494
Change-Id: I0292d7038d6510ca7c58af32b6003a1e4b121b00
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/250541
Reviewed-by: Hanna Silen <silen@webrtc.org>
Commit-Queue: Alessio Bazzica <alessiob@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#35910}
This commit is contained in:
Alessio Bazzica 2022-02-03 16:30:25 +01:00 committed by WebRTC LUCI CQ
parent b02e1acdaa
commit bab128555a
12 changed files with 81 additions and 64 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
modules/audio_processing/agc/BUILD.gn
View File

@ -27,6 +27,7 @@ rtc_library("agc") {
"..:apm_logging",
"..:audio_buffer",
"..:audio_frame_view",
"../../../api:array_view",
"../../../common_audio",
"../../../common_audio:common_audio_c",
"../../../rtc_base:checks",
@ -108,6 +109,7 @@ rtc_library("level_estimation") {
"utility.h",
]
deps = [
"../../../api:array_view",
"../../../rtc_base:checks",
"../vad",
]
@ -174,6 +176,7 @@ if (rtc_include_tests) {
":gain_control_interface",
":level_estimation",
"..:mocks",
"../../../api:array_view",
"../../../rtc_base:checks",
"../../../rtc_base:rtc_base_approved",
"../../../rtc_base:safe_conversions",

14
modules/audio_processing/agc/agc.cc
View File

@ -21,9 +21,11 @@
namespace webrtc {
namespace {
const int kDefaultLevelDbfs = -18;
const int kNumAnalysisFrames = 100;
const double kActivityThreshold = 0.3;
constexpr int kDefaultLevelDbfs = -18;
constexpr int kNumAnalysisFrames = 100;
constexpr double kActivityThreshold = 0.3;
constexpr int kNum10msFramesInOneSecond = 100;
constexpr int kMaxSampleRateHz = 384000;
} // namespace
@ -35,8 +37,10 @@ Agc::Agc()
Agc::~Agc() = default;
void Agc::Process(const int16_t* audio, size_t length, int sample_rate_hz) {
vad_.ProcessChunk(audio, length, sample_rate_hz);
void Agc::Process(rtc::ArrayView<const int16_t> audio) {
const int sample_rate_hz = audio.size() * kNum10msFramesInOneSecond;
RTC_DCHECK_LE(sample_rate_hz, kMaxSampleRateHz);
vad_.ProcessChunk(audio.data(), audio.size(), sample_rate_hz);
const std::vector<double>& rms = vad_.chunkwise_rms();
const std::vector<double>& probabilities =
vad_.chunkwise_voice_probabilities();

3
modules/audio_processing/agc/agc.h
View File

@ -13,6 +13,7 @@
#include <memory>
#include "api/array_view.h"
#include "modules/audio_processing/vad/voice_activity_detector.h"
namespace webrtc {
@ -26,7 +27,7 @@ class Agc {
// `audio` must be mono; in a multi-channel stream, provide the first (usually
// left) channel.
virtual void Process(const int16_t* audio, size_t length, int sample_rate_hz);
virtual void Process(rtc::ArrayView<const int16_t> audio);
// Retrieves the difference between the target RMS level and the current
// signal RMS level in dB. Returns true if an update is available and false

30
modules/audio_processing/agc/agc_manager_direct.cc
View File

@ -13,6 +13,7 @@
#include <algorithm>
#include <cmath>
#include "api/array_view.h"
#include "common_audio/include/audio_util.h"
#include "modules/audio_processing/agc/gain_control.h"
#include "modules/audio_processing/agc/gain_map_internal.h"
@ -204,9 +205,7 @@ void MonoAgc::Initialize() {
check_volume_on_next_process_ = true;
}
void MonoAgc::Process(const int16_t* audio,
size_t samples_per_channel,
int sample_rate_hz) {
void MonoAgc::Process(rtc::ArrayView<const int16_t> audio) {
new_compression_to_set_ = absl::nullopt;
if (check_volume_on_next_process_) {
@ -216,7 +215,7 @@ void MonoAgc::Process(const int16_t* audio,
CheckVolumeAndReset();
}
agc_->Process(audio, samples_per_channel, sample_rate_hz);
agc_->Process(audio);
UpdateGain();
if (!disable_digital_adaptive_) {
@ -447,7 +446,6 @@ AgcManagerDirect::AgcManagerDirect(
Agc* agc,
int startup_min_level,
int clipped_level_min,
int sample_rate_hz,
int clipped_level_step,
float clipped_ratio_threshold,
int clipped_wait_frames,
@ -456,7 +454,6 @@ AgcManagerDirect::AgcManagerDirect(
startup_min_level,
clipped_level_min,
/*disable_digital_adaptive*/ false,
sample_rate_hz,
clipped_level_step,
clipped_ratio_threshold,
clipped_wait_frames,
@ -471,7 +468,6 @@ AgcManagerDirect::AgcManagerDirect(
int startup_min_level,
int clipped_level_min,
bool disable_digital_adaptive,
int sample_rate_hz,
int clipped_level_step,
float clipped_ratio_threshold,
int clipped_wait_frames,
@ -479,7 +475,6 @@ AgcManagerDirect::AgcManagerDirect(
: data_dumper_(
new ApmDataDumper(rtc::AtomicOps::Increment(&instance_counter_))),
use_min_channel_level_(!UseMaxAnalogChannelLevel()),
sample_rate_hz_(sample_rate_hz),
num_capture_channels_(num_capture_channels),
disable_digital_adaptive_(disable_digital_adaptive),
frames_since_clipped_(clipped_wait_frames),
@ -652,27 +647,20 @@ void AgcManagerDirect::AnalyzePreProcess(const float* const* audio,
}
void AgcManagerDirect::Process(const AudioBuffer* audio) {
RTC_DCHECK(audio);
AggregateChannelLevels();
if (!capture_output_used_) {
return;
}
const size_t num_frames_per_band = audio->num_frames_per_band();
for (size_t ch = 0; ch < channel_agcs_.size(); ++ch) {
int16_t* audio_use = nullptr;
std::array<int16_t, AudioBuffer::kMaxSampleRate / 100> audio_data;
int num_frames_per_band;
if (audio) {
FloatS16ToS16(audio->split_bands_const_f(ch)[0],
audio->num_frames_per_band(), audio_data.data());
audio_use = audio_data.data();
num_frames_per_band = audio->num_frames_per_band();
} else {
// Only used for testing.
// TODO(peah): Change unittests to only allow on non-null audio input.
num_frames_per_band = 320;
}
channel_agcs_[ch]->Process(audio_use, num_frames_per_band, sample_rate_hz_);
int16_t* audio_use = audio_data.data();
FloatS16ToS16(audio->split_bands_const_f(ch)[0], num_frames_per_band,
audio_use);
channel_agcs_[ch]->Process({audio_use, num_frames_per_band});
new_compressions_to_set_[ch] = channel_agcs_[ch]->new_compression();
}

9
modules/audio_processing/agc/agc_manager_direct.h
View File

@ -14,6 +14,7 @@
#include <memory>
#include "absl/types/optional.h"
#include "api/array_view.h"
#include "modules/audio_processing/agc/agc.h"
#include "modules/audio_processing/agc/clipping_predictor.h"
#include "modules/audio_processing/agc/clipping_predictor_evaluator.h"
@ -47,7 +48,6 @@ class AgcManagerDirect final {
int startup_min_level,
int clipped_level_min,
bool disable_digital_adaptive,
int sample_rate_hz,
int clipped_level_step,
float clipped_ratio_threshold,
int clipped_wait_frames,
@ -72,7 +72,6 @@ class AgcManagerDirect final {
int stream_analog_level() const { return stream_analog_level_; }
void set_stream_analog_level(int level);
int num_channels() const { return num_capture_channels_; }
int sample_rate_hz() const { return sample_rate_hz_; }
// If available, returns a new compression gain for the digital gain control.
absl::optional<int> GetDigitalComressionGain();
@ -117,7 +116,6 @@ class AgcManagerDirect final {
Agc* agc,
int startup_min_level,
int clipped_level_min,
int sample_rate_hz,
int clipped_level_step,
float clipped_ratio_threshold,
int clipped_wait_frames,
@ -131,7 +129,6 @@ class AgcManagerDirect final {
std::unique_ptr<ApmDataDumper> data_dumper_;
static int instance_counter_;
const bool use_min_channel_level_;
const int sample_rate_hz_;
const int num_capture_channels_;
const bool disable_digital_adaptive_;
@ -171,9 +168,7 @@ class MonoAgc {
void HandleClipping(int clipped_level_step);
void Process(const int16_t* audio,
size_t samples_per_channel,
int sample_rate_hz);
void Process(rtc::ArrayView<const int16_t> audio);
void set_stream_analog_level(int level) { stream_analog_level_ = level; }
int stream_analog_level() const { return stream_analog_level_; }

68
modules/audio_processing/agc/agc_manager_direct_unittest.cc
View File

@ -10,6 +10,8 @@
#include "modules/audio_processing/agc/agc_manager_direct.h"
#include <limits>
#include "modules/audio_processing/agc/gain_control.h"
#include "modules/audio_processing/agc/mock_agc.h"
#include "modules/audio_processing/include/mock_audio_processing.h"
@ -69,7 +71,7 @@ std::unique_ptr<AgcManagerDirect> CreateAgcManagerDirect(
int clipped_wait_frames) {
return std::make_unique<AgcManagerDirect>(
/*num_capture_channels=*/1, startup_min_level, kClippedMin,
/*disable_digital_adaptive=*/true, kSampleRateHz, clipped_level_step,
/*disable_digital_adaptive=*/true, clipped_level_step,
clipped_ratio_threshold, clipped_wait_frames, ClippingPredictorConfig());
}
@ -81,7 +83,7 @@ std::unique_ptr<AgcManagerDirect> CreateAgcManagerDirect(
const ClippingPredictorConfig& clipping_cfg) {
return std::make_unique<AgcManagerDirect>(
/*num_capture_channels=*/1, startup_min_level, kClippedMin,
/*disable_digital_adaptive=*/true, kSampleRateHz, clipped_level_step,
/*disable_digital_adaptive=*/true, clipped_level_step,
clipped_ratio_threshold, clipped_wait_frames, clipping_cfg);
}
@ -112,6 +114,16 @@ void CallPreProcessAudioBuffer(int num_calls,
}
}
void WriteAudioBufferSamples(float samples_value, AudioBuffer& audio_buffer) {
RTC_DCHECK_GE(samples_value, std::numeric_limits<int16_t>::min());
RTC_DCHECK_LE(samples_value, std::numeric_limits<int16_t>::max());
for (size_t ch = 0; ch < audio_buffer.num_channels(); ++ch) {
for (size_t i = 0; i < audio_buffer.num_frames(); ++i) {
audio_buffer.channels()[ch][i] = samples_value;
}
}
}
} // namespace
class AgcManagerDirectTest : public ::testing::Test {
@ -121,11 +133,16 @@ class AgcManagerDirectTest : public ::testing::Test {
manager_(agc_,
kInitialVolume,
kClippedMin,
kSampleRateHz,
kClippedLevelStep,
kClippedRatioThreshold,
kClippedWaitFrames,
ClippingPredictorConfig()),
audio_buffer(kSampleRateHz,
kNumChannels,
kSampleRateHz,
kNumChannels,
kSampleRateHz,
kNumChannels),
audio(kNumChannels),
audio_data(kNumChannels * kSamplesPerChannel, 0.f) {
ExpectInitialize();
@ -134,6 +151,7 @@ class AgcManagerDirectTest : public ::testing::Test {
for (size_t ch = 0; ch < kNumChannels; ++ch) {
audio[ch] = &audio_data[ch * kSamplesPerChannel];
}
WriteAudioBufferSamples(/*samples_value=*/0.0f, audio_buffer);
}
void FirstProcess() {
@ -161,8 +179,8 @@ class AgcManagerDirectTest : public ::testing::Test {
void CallProcess(int num_calls) {
for (int i = 0; i < num_calls; ++i) {
EXPECT_CALL(*agc_, Process(_, _, _)).WillOnce(Return());
manager_.Process(nullptr);
EXPECT_CALL(*agc_, Process(_)).WillOnce(Return());
manager_.Process(&audio_buffer);
absl::optional<int> new_digital_gain =
manager_.GetDigitalComressionGain();
if (new_digital_gain) {
@ -209,6 +227,7 @@ class AgcManagerDirectTest : public ::testing::Test {
MockAgc* agc_;
MockGainControl gctrl_;
AgcManagerDirect manager_;
AudioBuffer audio_buffer;
std::vector<float*> audio;
std::vector<float> audio_data;
};
@ -452,7 +471,7 @@ TEST_F(AgcManagerDirectTest, CompressorReachesMinimum) {
TEST_F(AgcManagerDirectTest, NoActionWhileMuted) {
manager_.HandleCaptureOutputUsedChange(false);
manager_.Process(nullptr);
manager_.Process(&audio_buffer);
absl::optional<int> new_digital_gain = manager_.GetDigitalComressionGain();
if (new_digital_gain) {
gctrl_.set_compression_gain_db(*new_digital_gain);
@ -931,17 +950,20 @@ TEST(AgcManagerDirectStandaloneTest,
TEST(AgcManagerDirectStandaloneTest,
DisableClippingPredictorDoesNotLowerVolume) {
AudioBuffer audio_buffer(kSampleRateHz, kNumChannels, kSampleRateHz,
kNumChannels, kSampleRateHz, kNumChannels);
// TODO(bugs.webrtc.org/12874): Use designated initializers one fixed.
constexpr ClippingPredictorConfig kConfig{/*enabled=*/false};
AgcManagerDirect manager(new ::testing::NiceMock<MockAgc>(), kInitialVolume,
kClippedMin, kSampleRateHz, kClippedLevelStep,
kClippedMin, kClippedLevelStep,
kClippedRatioThreshold, kClippedWaitFrames, kConfig);
manager.Initialize();
manager.set_stream_analog_level(/*level=*/255);
EXPECT_FALSE(manager.clipping_predictor_enabled());
EXPECT_FALSE(manager.use_clipping_predictor_step());
EXPECT_EQ(manager.stream_analog_level(), 255);
manager.Process(nullptr);
manager.Process(&audio_buffer);
CallPreProcessAudioBuffer(/*num_calls=*/10, /*peak_ratio=*/0.99f, manager);
EXPECT_EQ(manager.stream_analog_level(), 255);
CallPreProcessAudioBuffer(/*num_calls=*/300, /*peak_ratio=*/0.99f, manager);
@ -952,20 +974,23 @@ TEST(AgcManagerDirectStandaloneTest,
TEST(AgcManagerDirectStandaloneTest,
UsedClippingPredictionsProduceLowerAnalogLevels) {
AudioBuffer audio_buffer(kSampleRateHz, kNumChannels, kSampleRateHz,
kNumChannels, kSampleRateHz, kNumChannels);
// TODO(bugs.webrtc.org/12874): Use designated initializers once fixed.
ClippingPredictorConfig config_with_prediction;
config_with_prediction.enabled = true;
config_with_prediction.use_predicted_step = true;
AgcManagerDirect manager_with_prediction(
new ::testing::NiceMock<MockAgc>(), kInitialVolume, kClippedMin,
kSampleRateHz, kClippedLevelStep, kClippedRatioThreshold,
kClippedWaitFrames, config_with_prediction);
kClippedLevelStep, kClippedRatioThreshold, kClippedWaitFrames,
config_with_prediction);
ClippingPredictorConfig config_without_prediction;
config_without_prediction.enabled = false;
AgcManagerDirect manager_without_prediction(
new ::testing::NiceMock<MockAgc>(), kInitialVolume, kClippedMin,
kSampleRateHz, kClippedLevelStep, kClippedRatioThreshold,
kClippedWaitFrames, config_without_prediction);
kClippedLevelStep, kClippedRatioThreshold, kClippedWaitFrames,
config_without_prediction);
manager_with_prediction.Initialize();
manager_without_prediction.Initialize();
constexpr int kInitialLevel = 255;
@ -974,8 +999,8 @@ TEST(AgcManagerDirectStandaloneTest,
constexpr float kZeroPeakRatio = 0.0f;
manager_with_prediction.set_stream_analog_level(kInitialLevel);
manager_without_prediction.set_stream_analog_level(kInitialLevel);
manager_with_prediction.Process(nullptr);
manager_without_prediction.Process(nullptr);
manager_with_prediction.Process(&audio_buffer);
manager_without_prediction.Process(&audio_buffer);
EXPECT_TRUE(manager_with_prediction.clipping_predictor_enabled());
EXPECT_FALSE(manager_without_prediction.clipping_predictor_enabled());
EXPECT_TRUE(manager_with_prediction.use_clipping_predictor_step());
@ -1044,20 +1069,23 @@ TEST(AgcManagerDirectStandaloneTest,
TEST(AgcManagerDirectStandaloneTest,
UnusedClippingPredictionsProduceEqualAnalogLevels) {
AudioBuffer audio_buffer(kSampleRateHz, kNumChannels, kSampleRateHz,
kNumChannels, kSampleRateHz, kNumChannels);
// TODO(bugs.webrtc.org/12874): Use designated initializers once fixed.
ClippingPredictorConfig config_with_prediction;
config_with_prediction.enabled = true;
config_with_prediction.use_predicted_step = false;
AgcManagerDirect manager_with_prediction(
new ::testing::NiceMock<MockAgc>(), kInitialVolume, kClippedMin,
kSampleRateHz, kClippedLevelStep, kClippedRatioThreshold,
kClippedWaitFrames, config_with_prediction);
kClippedLevelStep, kClippedRatioThreshold, kClippedWaitFrames,
config_with_prediction);
ClippingPredictorConfig config_without_prediction;
config_without_prediction.enabled = false;
AgcManagerDirect manager_without_prediction(
new ::testing::NiceMock<MockAgc>(), kInitialVolume, kClippedMin,
kSampleRateHz, kClippedLevelStep, kClippedRatioThreshold,
kClippedWaitFrames, config_without_prediction);
kClippedLevelStep, kClippedRatioThreshold, kClippedWaitFrames,
config_without_prediction);
constexpr int kInitialLevel = 255;
constexpr float kClippingPeakRatio = 1.0f;
constexpr float kCloseToClippingPeakRatio = 0.99f;
@ -1066,8 +1094,8 @@ TEST(AgcManagerDirectStandaloneTest,
manager_without_prediction.Initialize();
manager_with_prediction.set_stream_analog_level(kInitialLevel);
manager_without_prediction.set_stream_analog_level(kInitialLevel);
manager_with_prediction.Process(nullptr);
manager_without_prediction.Process(nullptr);
manager_with_prediction.Process(&audio_buffer);
manager_without_prediction.Process(&audio_buffer);
EXPECT_TRUE(manager_with_prediction.clipping_predictor_enabled());
EXPECT_FALSE(manager_without_prediction.clipping_predictor_enabled());
EXPECT_FALSE(manager_with_prediction.use_clipping_predictor_step());

6
modules/audio_processing/agc/mock_agc.h
View File

@ -11,6 +11,7 @@
#ifndef MODULES_AUDIO_PROCESSING_AGC_MOCK_AGC_H_
#define MODULES_AUDIO_PROCESSING_AGC_MOCK_AGC_H_
#include "api/array_view.h"
#include "modules/audio_processing/agc/agc.h"
#include "test/gmock.h"
@ -19,10 +20,7 @@ namespace webrtc {
class MockAgc : public Agc {
public:
virtual ~MockAgc() {}
MOCK_METHOD(void,
Process,
(const int16_t* audio, size_t length, int sample_rate_hz),
(override));
MOCK_METHOD(void, Process, (rtc::ArrayView<const int16_t> audio), (override));
MOCK_METHOD(bool, GetRmsErrorDb, (int* error), (override));
MOCK_METHOD(void, Reset, (), (override));
MOCK_METHOD(int, set_target_level_dbfs, (int level), (override));

5
modules/audio_processing/audio_processing_impl.cc
View File

@ -1858,9 +1858,7 @@ void AudioProcessingImpl::InitializeGainController1() {
if (!submodules_.agc_manager.get() ||
submodules_.agc_manager->num_channels() !=
static_cast<int>(num_proc_channels()) ||
submodules_.agc_manager->sample_rate_hz() !=
capture_nonlocked_.split_rate) {
static_cast<int>(num_proc_channels())) {
int stream_analog_level = -1;
const bool re_creation = !!submodules_.agc_manager;
if (re_creation) {
@ -1872,7 +1870,6 @@ void AudioProcessingImpl::InitializeGainController1() {
config_.gain_controller1.analog_gain_controller.clipped_level_min,
!config_.gain_controller1.analog_gain_controller
.enable_digital_adaptive,
capture_nonlocked_.split_rate,
config_.gain_controller1.analog_gain_controller.clipped_level_step,
config_.gain_controller1.analog_gain_controller.clipped_ratio_threshold,
config_.gain_controller1.analog_gain_controller.clipped_wait_frames,

1
modules/audio_processing/audio_processing_impl.h
View File

@ -18,6 +18,7 @@
#include <string>
#include <vector>
#include "api/array_view.h"
#include "api/function_view.h"
#include "modules/audio_processing/aec3/echo_canceller3.h"
#include "modules/audio_processing/agc/agc_manager_direct.h"

3
modules/audio_processing/transient/transient_suppression_test.cc
View File

@ -191,8 +191,7 @@ void void_main() {
in_file, audio_buffer_size, absl::GetFlag(FLAGS_num_channels),
audio_buffer_i.get(), detection_file, detection_buffer_size,
detection_buffer.get(), reference_file, reference_buffer.get())) {
agc.Process(audio_buffer_i.get(), static_cast<int>(audio_buffer_size),
absl::GetFlag(FLAGS_sample_rate_hz));
agc.Process({audio_buffer_i.get(), audio_buffer_size});
for (size_t i = 0;
i < absl::GetFlag(FLAGS_num_channels) * audio_buffer_size; ++i) {

1
modules/audio_processing/vad/voice_activity_detector.cc
View File

@ -38,6 +38,7 @@ void VoiceActivityDetector::ProcessChunk(const int16_t* audio,
size_t length,
int sample_rate_hz) {
RTC_DCHECK_EQ(length, sample_rate_hz / 100);
// TODO(bugs.webrtc.org/7494): Remove resampling and force 16 kHz audio.
// Resample to the required rate.
const int16_t* resampled_ptr = audio;
if (sample_rate_hz != kSampleRateHz) {

2
modules/audio_processing/vad/voice_activity_detector.h
View File

@ -33,6 +33,8 @@ class VoiceActivityDetector {
~VoiceActivityDetector();
// Processes each audio chunk and estimates the voice probability.
// TODO(bugs.webrtc.org/7494): Switch to rtc::ArrayView and remove
// `sample_rate_hz`.
void ProcessChunk(const int16_t* audio, size_t length, int sample_rate_hz);
// Returns a vector of voice probabilities for each chunk. It can be empty for