Fix errors in AEC3 JSON parsing
Some issues were surfaced during testing:
- Config validation clamping silently passed NaNs
- Config validation only fixed the first out-of-bounds parameter, and
not any subsequent ones
- Config validation did not check all values in the config
- use_stationarity_properties_at_init is misspelled in JSON parsing
These changes are identical to those in this CL:
https://webrtc-review.googlesource.com/c/src/+/107120
Bug: webrtc:9535
Change-Id: I36c5e7c69ffdc2c0c24a9be86ccb1df59683c0fe
Reviewed-on: https://webrtc-review.googlesource.com/c/107640
Reviewed-by: Per Åhgren <peah@webrtc.org>
Commit-Queue: Sam Zackrisson <saza@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#25318}diff --git a/api/audio/echo_canceller3_config.cc b/api/audio/echo_canceller3_config.cc
index 3567b5b..29d0b9a 100644
--- a/api/audio/echo_canceller3_config.cc
+++ b/api/audio/echo_canceller3_config.cc
@@ -10,6 +10,7 @@
#include "api/audio/echo_canceller3_config.h"
#include <algorithm>
+#include <cmath>
#include "rtc_base/checks.h"
#include "rtc_base/numerics/safe_minmax.h"
@@ -18,6 +19,7 @@
namespace {
bool Limit(float* value, float min, float max) {
float clamped = rtc::SafeClamp(*value, min, max);
+ clamped = std::isfinite(clamped) ? clamped : min;
bool res = *value == clamped;
*value = clamped;
return res;
@@ -85,164 +87,161 @@
c->delay.down_sampling_factor = 4;
res = false;
}
-
- if (c->delay.num_filters == 0) {
- c->delay.num_filters = 1;
- res = false;
- }
- if (c->delay.api_call_jitter_blocks == 0) {
- c->delay.api_call_jitter_blocks = 1;
- res = false;
- }
-
- if (c->delay.api_call_jitter_blocks == 0) {
- c->delay.api_call_jitter_blocks = 1;
- res = false;
- }
if (c->delay.delay_headroom_blocks <= 1 &&
c->delay.hysteresis_limit_1_blocks == 1) {
c->delay.hysteresis_limit_1_blocks = 0;
res = false;
}
- res = res && Limit(&c->delay.delay_estimate_smoothing, 0.f, 1.f);
- res = res && Limit(&c->delay.delay_candidate_detection_threshold, 0.f, 1.f);
- res = res && Limit(&c->delay.delay_selection_thresholds.initial, 1, 250);
- res = res && Limit(&c->delay.delay_selection_thresholds.converged, 1, 250);
+ res = res & Limit(&c->delay.default_delay, 0, 5000);
+ res = res & Limit(&c->delay.num_filters, 0, 5000);
+ res = res & Limit(&c->delay.api_call_jitter_blocks, 1, 5000);
+ res = res & Limit(&c->delay.min_echo_path_delay_blocks, 0, 5000);
+ res = res & Limit(&c->delay.delay_headroom_blocks, 0, 5000);
+ res = res & Limit(&c->delay.hysteresis_limit_1_blocks, 0, 5000);
+ res = res & Limit(&c->delay.hysteresis_limit_2_blocks, 0, 5000);
+ res = res & Limit(&c->delay.skew_hysteresis_blocks, 0, 5000);
+ res = res & Limit(&c->delay.fixed_capture_delay_samples, 0, 5000);
+ res = res & Limit(&c->delay.delay_estimate_smoothing, 0.f, 1.f);
+ res = res & Limit(&c->delay.delay_candidate_detection_threshold, 0.f, 1.f);
+ res = res & Limit(&c->delay.delay_selection_thresholds.initial, 1, 250);
+ res = res & Limit(&c->delay.delay_selection_thresholds.converged, 1, 250);
- res = res && Limit(&c->filter.main.length_blocks, 1, 50);
- res = res && Limit(&c->filter.main.leakage_converged, 0.f, 1000.f);
- res = res && Limit(&c->filter.main.leakage_diverged, 0.f, 1000.f);
- res = res && Limit(&c->filter.main.error_floor, 0.f, 1000.f);
- res = res && Limit(&c->filter.main.noise_gate, 0.f, 100000000.f);
+ res = res & Limit(&c->filter.main.length_blocks, 1, 50);
+ res = res & Limit(&c->filter.main.leakage_converged, 0.f, 1000.f);
+ res = res & Limit(&c->filter.main.leakage_diverged, 0.f, 1000.f);
+ res = res & Limit(&c->filter.main.error_floor, 0.f, 1000.f);
+ res = res & Limit(&c->filter.main.error_ceil, 0.f, 100000000.f);
+ res = res & Limit(&c->filter.main.noise_gate, 0.f, 100000000.f);
- res = res && Limit(&c->filter.main_initial.length_blocks, 1, 50);
- res = res && Limit(&c->filter.main_initial.leakage_converged, 0.f, 1000.f);
- res = res && Limit(&c->filter.main_initial.leakage_diverged, 0.f, 1000.f);
- res = res && Limit(&c->filter.main_initial.error_floor, 0.f, 1000.f);
- res = res && Limit(&c->filter.main_initial.noise_gate, 0.f, 100000000.f);
+ res = res & Limit(&c->filter.main_initial.length_blocks, 1, 50);
+ res = res & Limit(&c->filter.main_initial.leakage_converged, 0.f, 1000.f);
+ res = res & Limit(&c->filter.main_initial.leakage_diverged, 0.f, 1000.f);
+ res = res & Limit(&c->filter.main_initial.error_floor, 0.f, 1000.f);
+ res = res & Limit(&c->filter.main_initial.error_ceil, 0.f, 100000000.f);
+ res = res & Limit(&c->filter.main_initial.noise_gate, 0.f, 100000000.f);
if (c->filter.main.length_blocks < c->filter.main_initial.length_blocks) {
c->filter.main_initial.length_blocks = c->filter.main.length_blocks;
res = false;
}
- res = res && Limit(&c->filter.shadow.length_blocks, 1, 50);
- res = res && Limit(&c->filter.shadow.rate, 0.f, 1.f);
- res = res && Limit(&c->filter.shadow.noise_gate, 0.f, 100000000.f);
+ res = res & Limit(&c->filter.shadow.length_blocks, 1, 50);
+ res = res & Limit(&c->filter.shadow.rate, 0.f, 1.f);
+ res = res & Limit(&c->filter.shadow.noise_gate, 0.f, 100000000.f);
- res = res && Limit(&c->filter.shadow_initial.length_blocks, 1, 50);
- res = res && Limit(&c->filter.shadow_initial.rate, 0.f, 1.f);
- res = res && Limit(&c->filter.shadow_initial.noise_gate, 0.f, 100000000.f);
+ res = res & Limit(&c->filter.shadow_initial.length_blocks, 1, 50);
+ res = res & Limit(&c->filter.shadow_initial.rate, 0.f, 1.f);
+ res = res & Limit(&c->filter.shadow_initial.noise_gate, 0.f, 100000000.f);
if (c->filter.shadow.length_blocks < c->filter.shadow_initial.length_blocks) {
c->filter.shadow_initial.length_blocks = c->filter.shadow.length_blocks;
res = false;
}
- res = res && Limit(&c->filter.config_change_duration_blocks, 0, 100000);
- res = res && Limit(&c->filter.initial_state_seconds, 0.f, 100.f);
+ res = res & Limit(&c->filter.config_change_duration_blocks, 0, 100000);
+ res = res & Limit(&c->filter.initial_state_seconds, 0.f, 100.f);
- res = res && Limit(&c->erle.min, 1.f, 100000.f);
- res = res && Limit(&c->erle.max_l, 1.f, 100000.f);
- res = res && Limit(&c->erle.max_h, 1.f, 100000.f);
+ res = res & Limit(&c->erle.min, 1.f, 100000.f);
+ res = res & Limit(&c->erle.max_l, 1.f, 100000.f);
+ res = res & Limit(&c->erle.max_h, 1.f, 100000.f);
if (c->erle.min > c->erle.max_l || c->erle.min > c->erle.max_h) {
c->erle.min = std::min(c->erle.max_l, c->erle.max_h);
res = false;
}
- res = res && Limit(&c->ep_strength.lf, 0.f, 1000000.f);
- res = res && Limit(&c->ep_strength.mf, 0.f, 1000000.f);
- res = res && Limit(&c->ep_strength.hf, 0.f, 1000000.f);
- res = res && Limit(&c->ep_strength.default_len, 0.f, 1.f);
-
- res = res &&
- Limit(&c->echo_audibility.low_render_limit, 0.f, 32768.f * 32768.f);
- res = res &&
- Limit(&c->echo_audibility.normal_render_limit, 0.f, 32768.f * 32768.f);
- res = res && Limit(&c->echo_audibility.floor_power, 0.f, 32768.f * 32768.f);
- res = res && Limit(&c->echo_audibility.audibility_threshold_lf, 0.f,
- 32768.f * 32768.f);
- res = res && Limit(&c->echo_audibility.audibility_threshold_mf, 0.f,
- 32768.f * 32768.f);
- res = res && Limit(&c->echo_audibility.audibility_threshold_hf, 0.f,
- 32768.f * 32768.f);
-
- res = res &&
- Limit(&c->render_levels.active_render_limit, 0.f, 32768.f * 32768.f);
- res = res && Limit(&c->render_levels.poor_excitation_render_limit, 0.f,
- 32768.f * 32768.f);
- res = res && Limit(&c->render_levels.poor_excitation_render_limit_ds8, 0.f,
- 32768.f * 32768.f);
+ res = res & Limit(&c->ep_strength.lf, 0.f, 1000000.f);
+ res = res & Limit(&c->ep_strength.mf, 0.f, 1000000.f);
+ res = res & Limit(&c->ep_strength.hf, 0.f, 1000000.f);
+ res = res & Limit(&c->ep_strength.default_len, 0.f, 1.f);
res =
- res && Limit(&c->echo_removal_control.gain_rampup.initial_gain, 0.f, 1.f);
- res = res && Limit(&c->echo_removal_control.gain_rampup.first_non_zero_gain,
- 0.f, 1.f);
- res = res && Limit(&c->echo_removal_control.gain_rampup.non_zero_gain_blocks,
- 0, 100000);
- res = res &&
+ res & Limit(&c->echo_audibility.low_render_limit, 0.f, 32768.f * 32768.f);
+ res = res &
+ Limit(&c->echo_audibility.normal_render_limit, 0.f, 32768.f * 32768.f);
+ res = res & Limit(&c->echo_audibility.floor_power, 0.f, 32768.f * 32768.f);
+ res = res & Limit(&c->echo_audibility.audibility_threshold_lf, 0.f,
+ 32768.f * 32768.f);
+ res = res & Limit(&c->echo_audibility.audibility_threshold_mf, 0.f,
+ 32768.f * 32768.f);
+ res = res & Limit(&c->echo_audibility.audibility_threshold_hf, 0.f,
+ 32768.f * 32768.f);
+
+ res = res &
+ Limit(&c->render_levels.active_render_limit, 0.f, 32768.f * 32768.f);
+ res = res & Limit(&c->render_levels.poor_excitation_render_limit, 0.f,
+ 32768.f * 32768.f);
+ res = res & Limit(&c->render_levels.poor_excitation_render_limit_ds8, 0.f,
+ 32768.f * 32768.f);
+
+ res =
+ res & Limit(&c->echo_removal_control.gain_rampup.initial_gain, 0.f, 1.f);
+ res = res & Limit(&c->echo_removal_control.gain_rampup.first_non_zero_gain,
+ 0.f, 1.f);
+ res = res & Limit(&c->echo_removal_control.gain_rampup.non_zero_gain_blocks,
+ 0, 100000);
+ res = res &
Limit(&c->echo_removal_control.gain_rampup.full_gain_blocks, 0, 100000);
- res = res && Limit(&c->echo_model.noise_floor_hold, 0, 1000);
- res = res && Limit(&c->echo_model.min_noise_floor_power, 0, 2000000.f);
- res = res && Limit(&c->echo_model.stationary_gate_slope, 0, 1000000.f);
- res = res && Limit(&c->echo_model.noise_gate_power, 0, 1000000.f);
- res = res && Limit(&c->echo_model.noise_gate_slope, 0, 1000000.f);
- res = res && Limit(&c->echo_model.render_pre_window_size, 0, 100);
- res = res && Limit(&c->echo_model.render_post_window_size, 0, 100);
- res = res && Limit(&c->echo_model.render_pre_window_size_init, 0, 100);
- res = res && Limit(&c->echo_model.render_post_window_size_init, 0, 100);
- res = res && Limit(&c->echo_model.nonlinear_hold, 0, 100);
- res = res && Limit(&c->echo_model.nonlinear_release, 0, 1.f);
+ res = res & Limit(&c->echo_model.noise_floor_hold, 0, 1000);
+ res = res & Limit(&c->echo_model.min_noise_floor_power, 0, 2000000.f);
+ res = res & Limit(&c->echo_model.stationary_gate_slope, 0, 1000000.f);
+ res = res & Limit(&c->echo_model.noise_gate_power, 0, 1000000.f);
+ res = res & Limit(&c->echo_model.noise_gate_slope, 0, 1000000.f);
+ res = res & Limit(&c->echo_model.render_pre_window_size, 0, 100);
+ res = res & Limit(&c->echo_model.render_post_window_size, 0, 100);
+ res = res & Limit(&c->echo_model.render_pre_window_size_init, 0, 100);
+ res = res & Limit(&c->echo_model.render_post_window_size_init, 0, 100);
+ res = res & Limit(&c->echo_model.nonlinear_hold, 0, 100);
+ res = res & Limit(&c->echo_model.nonlinear_release, 0, 1.f);
- res = res &&
+ res = res & Limit(&c->suppressor.nearend_average_blocks, 1, 5000);
+
+ res = res &
Limit(&c->suppressor.normal_tuning.mask_lf.enr_transparent, 0.f, 100.f);
- res = res &&
+ res = res &
Limit(&c->suppressor.normal_tuning.mask_lf.enr_suppress, 0.f, 100.f);
- res = res &&
+ res = res &
Limit(&c->suppressor.normal_tuning.mask_lf.emr_transparent, 0.f, 100.f);
- res = res &&
+ res = res &
Limit(&c->suppressor.normal_tuning.mask_hf.enr_transparent, 0.f, 100.f);
- res = res &&
+ res = res &
Limit(&c->suppressor.normal_tuning.mask_hf.enr_suppress, 0.f, 100.f);
- res = res &&
+ res = res &
Limit(&c->suppressor.normal_tuning.mask_hf.emr_transparent, 0.f, 100.f);
- res = res && Limit(&c->suppressor.normal_tuning.max_inc_factor, 0.f, 100.f);
- res =
- res && Limit(&c->suppressor.normal_tuning.max_dec_factor_lf, 0.f, 100.f);
+ res = res & Limit(&c->suppressor.normal_tuning.max_inc_factor, 0.f, 100.f);
+ res = res & Limit(&c->suppressor.normal_tuning.max_dec_factor_lf, 0.f, 100.f);
- res = res && Limit(&c->suppressor.nearend_tuning.mask_lf.enr_transparent, 0.f,
- 100.f);
- res = res &&
+ res = res & Limit(&c->suppressor.nearend_tuning.mask_lf.enr_transparent, 0.f,
+ 100.f);
+ res = res &
Limit(&c->suppressor.nearend_tuning.mask_lf.enr_suppress, 0.f, 100.f);
- res = res && Limit(&c->suppressor.nearend_tuning.mask_lf.emr_transparent, 0.f,
- 100.f);
- res = res && Limit(&c->suppressor.nearend_tuning.mask_hf.enr_transparent, 0.f,
- 100.f);
- res = res &&
+ res = res & Limit(&c->suppressor.nearend_tuning.mask_lf.emr_transparent, 0.f,
+ 100.f);
+ res = res & Limit(&c->suppressor.nearend_tuning.mask_hf.enr_transparent, 0.f,
+ 100.f);
+ res = res &
Limit(&c->suppressor.nearend_tuning.mask_hf.enr_suppress, 0.f, 100.f);
- res = res && Limit(&c->suppressor.nearend_tuning.mask_hf.emr_transparent, 0.f,
- 100.f);
- res = res && Limit(&c->suppressor.nearend_tuning.max_inc_factor, 0.f, 100.f);
+ res = res & Limit(&c->suppressor.nearend_tuning.mask_hf.emr_transparent, 0.f,
+ 100.f);
+ res = res & Limit(&c->suppressor.nearend_tuning.max_inc_factor, 0.f, 100.f);
res =
- res && Limit(&c->suppressor.nearend_tuning.max_dec_factor_lf, 0.f, 100.f);
+ res & Limit(&c->suppressor.nearend_tuning.max_dec_factor_lf, 0.f, 100.f);
- res = res && Limit(&c->suppressor.dominant_nearend_detection.enr_threshold,
- 0.f, 1000000.f);
- res = res && Limit(&c->suppressor.dominant_nearend_detection.snr_threshold,
- 0.f, 1000000.f);
- res = res && Limit(&c->suppressor.dominant_nearend_detection.hold_duration, 0,
- 10000);
- res =
- res && Limit(&c->suppressor.dominant_nearend_detection.trigger_threshold,
- 0, 10000);
+ res = res & Limit(&c->suppressor.dominant_nearend_detection.enr_threshold,
+ 0.f, 1000000.f);
+ res = res & Limit(&c->suppressor.dominant_nearend_detection.snr_threshold,
+ 0.f, 1000000.f);
+ res = res & Limit(&c->suppressor.dominant_nearend_detection.hold_duration, 0,
+ 10000);
+ res = res & Limit(&c->suppressor.dominant_nearend_detection.trigger_threshold,
+ 0, 10000);
- res = res && Limit(&c->suppressor.high_bands_suppression.enr_threshold, 0.f,
- 1000000.f);
- res = res && Limit(&c->suppressor.high_bands_suppression.max_gain_during_echo,
- 0.f, 1.f);
+ res = res & Limit(&c->suppressor.high_bands_suppression.enr_threshold, 0.f,
+ 1000000.f);
+ res = res & Limit(&c->suppressor.high_bands_suppression.max_gain_during_echo,
+ 0.f, 1.f);
- res = res && Limit(&c->suppressor.floor_first_increase, 0.f, 1000000.f);
+ res = res & Limit(&c->suppressor.floor_first_increase, 0.f, 1000000.f);
return res;
}
diff --git a/api/audio/echo_canceller3_config.h b/api/audio/echo_canceller3_config.h
index 5bb5ece..5eb2fd2 100644
--- a/api/audio/echo_canceller3_config.h
+++ b/api/audio/echo_canceller3_config.h
@@ -19,7 +19,7 @@
// Configuration struct for EchoCanceller3
struct RTC_EXPORT EchoCanceller3Config {
- // Checks and updates the parameters in a config to lie within reasonable
+ // Checks and updates the config parameters to lie within (mostly) reasonable
// ranges. Returns true if and only of the config did not need to be changed.
static bool Validate(EchoCanceller3Config* config);
diff --git a/api/audio/echo_canceller3_config_json.cc b/api/audio/echo_canceller3_config_json.cc
index 2b6a329..bb44162 100644
--- a/api/audio/echo_canceller3_config_json.cc
+++ b/api/audio/echo_canceller3_config_json.cc
@@ -30,6 +30,7 @@
RTC_DCHECK(param);
int v;
if (rtc::GetIntFromJsonObject(root, param_name, &v)) {
+ RTC_DCHECK_GE(v, 0);
*param = v;
}
}
@@ -216,7 +217,7 @@
&cfg.echo_audibility.audibility_threshold_hf);
ReadParam(section, "use_stationary_properties",
&cfg.echo_audibility.use_stationary_properties);
- ReadParam(section, "use_stationary_properties_at_init",
+ ReadParam(section, "use_stationarity_properties_at_init",
&cfg.echo_audibility.use_stationarity_properties_at_init);
}
