Make AEC3 json parsing code testonly
Reasons:
- the code is no longer used in Chrome
- it is conceptually weird for WebRTC to have JSON parsing in its API
- there are concerns around the reliability of the underlying JSON library
Additionally, this CL removes the rtc_json "poisonous" attribute: the scheme is incompatible and redundant with testonly.
Bug: webrtc:1493351
Change-Id: I0b621b0e3f183df7315919d9c89242fbe387928f
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/325062
Reviewed-by: Per Åhgren <peah@webrtc.org>
Reviewed-by: Mirko Bonadei <mbonadei@webrtc.org>
Commit-Queue: Sam Zackrisson <saza@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#41014}
diff --git a/BUILD.gn b/BUILD.gn
index cc86183..7e8325e 100644
--- a/BUILD.gn
+++ b/BUILD.gn
@@ -806,8 +806,5 @@
group("poison_default_echo_detector") {
}
-group("poison_rtc_json") {
-}
-
group("poison_software_video_codecs") {
}
diff --git a/api/audio/BUILD.gn b/api/audio/BUILD.gn
index 4832751..0ecab64 100644
--- a/api/audio/BUILD.gn
+++ b/api/audio/BUILD.gn
@@ -55,24 +55,6 @@
]
}
-rtc_library("aec3_config_json") {
- visibility = [ "*" ]
- allow_poison = [ "rtc_json" ]
- sources = [
- "echo_canceller3_config_json.cc",
- "echo_canceller3_config_json.h",
- ]
- deps = [
- ":aec3_config",
- "../../rtc_base:checks",
- "../../rtc_base:logging",
- "../../rtc_base:rtc_json",
- "../../rtc_base:stringutils",
- "../../rtc_base/system:rtc_export",
- ]
- absl_deps = [ "//third_party/abseil-cpp/absl/strings" ]
-}
-
rtc_library("aec3_factory") {
visibility = [ "*" ]
configs += [ "../../modules/audio_processing:apm_debug_dump" ]
diff --git a/api/audio/test/BUILD.gn b/api/audio/test/BUILD.gn
index dfe8c32..8d5822f 100644
--- a/api/audio/test/BUILD.gn
+++ b/api/audio/test/BUILD.gn
@@ -17,13 +17,12 @@
testonly = true
sources = [
"audio_frame_unittest.cc",
- "echo_canceller3_config_json_unittest.cc",
"echo_canceller3_config_unittest.cc",
]
deps = [
"..:aec3_config",
- "..:aec3_config_json",
"..:audio_frame_api",
+ "../../../modules/audio_processing:aec3_config_json",
"../../../test:test_support",
]
}
diff --git a/api/audio/test/echo_canceller3_config_unittest.cc b/api/audio/test/echo_canceller3_config_unittest.cc
index 91312a0..da02558 100644
--- a/api/audio/test/echo_canceller3_config_unittest.cc
+++ b/api/audio/test/echo_canceller3_config_unittest.cc
@@ -10,7 +10,7 @@
#include "api/audio/echo_canceller3_config.h"
-#include "api/audio/echo_canceller3_config_json.h"
+#include "modules/audio_processing/test/echo_canceller3_config_json.h"
#include "test/gtest.h"
namespace webrtc {
diff --git a/modules/audio_processing/BUILD.gn b/modules/audio_processing/BUILD.gn
index 64e83a0..2b81427 100644
--- a/modules/audio_processing/BUILD.gn
+++ b/modules/audio_processing/BUILD.gn
@@ -371,10 +371,12 @@
"echo_control_mobile_unittest.cc",
"gain_controller2_unittest.cc",
"splitting_filter_unittest.cc",
+ "test/echo_canceller3_config_json_unittest.cc",
"test/fake_recording_device_unittest.cc",
]
deps = [
+ ":aec3_config_json",
":analog_mic_simulation",
":api",
":apm_logging",
@@ -558,6 +560,7 @@
]
deps = [
+ ":aec3_config_json",
":analog_mic_simulation",
":api",
":apm_logging",
@@ -566,7 +569,6 @@
":audioproc_protobuf_utils",
":audioproc_test_utils",
":runtime_settings_protobuf_utils",
- "../../api/audio:aec3_config_json",
"../../api/audio:aec3_factory",
"../../api/audio:echo_detector_creator",
"../../common_audio",
@@ -675,3 +677,21 @@
"//third_party/abseil-cpp/absl/types:optional",
]
}
+
+rtc_library("aec3_config_json") {
+ visibility = [ "*" ]
+ testonly = true
+ sources = [
+ "test/echo_canceller3_config_json.cc",
+ "test/echo_canceller3_config_json.h",
+ ]
+ deps = [
+ "../../api/audio:aec3_config",
+ "../../rtc_base:checks",
+ "../../rtc_base:logging",
+ "../../rtc_base:rtc_json",
+ "../../rtc_base:stringutils",
+ "../../rtc_base/system:rtc_export",
+ ]
+ absl_deps = [ "//third_party/abseil-cpp/absl/strings" ]
+}
diff --git a/modules/audio_processing/test/audio_processing_simulator.cc b/modules/audio_processing/test/audio_processing_simulator.cc
index 7497d49..7bd6da0 100644
--- a/modules/audio_processing/test/audio_processing_simulator.cc
+++ b/modules/audio_processing/test/audio_processing_simulator.cc
@@ -19,13 +19,13 @@
#include <vector>
#include "absl/strings/string_view.h"
-#include "api/audio/echo_canceller3_config_json.h"
#include "api/audio/echo_canceller3_factory.h"
#include "api/audio/echo_detector_creator.h"
#include "modules/audio_processing/aec_dump/aec_dump_factory.h"
#include "modules/audio_processing/echo_control_mobile_impl.h"
#include "modules/audio_processing/include/audio_processing.h"
#include "modules/audio_processing/logging/apm_data_dumper.h"
+#include "modules/audio_processing/test/echo_canceller3_config_json.h"
#include "modules/audio_processing/test/fake_recording_device.h"
#include "rtc_base/checks.h"
#include "rtc_base/logging.h"
diff --git a/api/audio/echo_canceller3_config_json.cc b/modules/audio_processing/test/echo_canceller3_config_json.cc
similarity index 98%
rename from api/audio/echo_canceller3_config_json.cc
rename to modules/audio_processing/test/echo_canceller3_config_json.cc
index 96e45ff..d05e284 100644
--- a/api/audio/echo_canceller3_config_json.cc
+++ b/modules/audio_processing/test/echo_canceller3_config_json.cc
@@ -7,7 +7,7 @@
* in the file PATENTS. All contributing project authors may
* be found in the AUTHORS file in the root of the source tree.
*/
-#include "api/audio/echo_canceller3_config_json.h"
+#include "modules/audio_processing/test/echo_canceller3_config_json.h"
#include <stddef.h>
@@ -429,13 +429,6 @@
}
}
-EchoCanceller3Config Aec3ConfigFromJsonString(absl::string_view json_string) {
- EchoCanceller3Config cfg;
- bool not_used;
- Aec3ConfigFromJsonString(json_string, &cfg, ¬_used);
- return cfg;
-}
-
std::string Aec3ConfigToJsonString(const EchoCanceller3Config& config) {
rtc::StringBuilder ost;
ost << "{";
diff --git a/api/audio/echo_canceller3_config_json.h b/modules/audio_processing/test/echo_canceller3_config_json.h
similarity index 64%
rename from api/audio/echo_canceller3_config_json.h
rename to modules/audio_processing/test/echo_canceller3_config_json.h
index ecee954..6637c8f 100644
--- a/api/audio/echo_canceller3_config_json.h
+++ b/modules/audio_processing/test/echo_canceller3_config_json.h
@@ -8,14 +8,13 @@
* be found in the AUTHORS file in the root of the source tree.
*/
-#ifndef API_AUDIO_ECHO_CANCELLER3_CONFIG_JSON_H_
-#define API_AUDIO_ECHO_CANCELLER3_CONFIG_JSON_H_
+#ifndef MODULES_AUDIO_PROCESSING_TEST_ECHO_CANCELLER3_CONFIG_JSON_H_
+#define MODULES_AUDIO_PROCESSING_TEST_ECHO_CANCELLER3_CONFIG_JSON_H_
#include <string>
#include "absl/strings/string_view.h"
#include "api/audio/echo_canceller3_config.h"
-#include "rtc_base/system/rtc_export.h"
namespace webrtc {
// Parses a JSON-encoded string into an Aec3 config. Fields corresponds to
@@ -23,23 +22,15 @@
// "aec3". Produces default config values for anything that cannot be parsed
// from the string. If any error was found in the parsing, parsing_successful is
// set to false.
-RTC_EXPORT void Aec3ConfigFromJsonString(absl::string_view json_string,
+void Aec3ConfigFromJsonString(absl::string_view json_string,
EchoCanceller3Config* config,
bool* parsing_successful);
-// To be deprecated.
-// Parses a JSON-encoded string into an Aec3 config. Fields corresponds to
-// substruct names, with the addition that there must be a top-level node
-// "aec3". Returns default config values for anything that cannot be parsed from
-// the string.
-RTC_EXPORT EchoCanceller3Config
-Aec3ConfigFromJsonString(absl::string_view json_string);
-
// Encodes an Aec3 config in JSON format. Fields corresponds to substruct names,
// with the addition that the top-level node is named "aec3".
-RTC_EXPORT std::string Aec3ConfigToJsonString(
+std::string Aec3ConfigToJsonString(
const EchoCanceller3Config& config);
} // namespace webrtc
-#endif // API_AUDIO_ECHO_CANCELLER3_CONFIG_JSON_H_
+#endif // MODULES_AUDIO_PROCESSING_TEST_ECHO_CANCELLER3_CONFIG_JSON_H_
diff --git a/api/audio/test/echo_canceller3_config_json_unittest.cc b/modules/audio_processing/test/echo_canceller3_config_json_unittest.cc
similarity index 94%
rename from api/audio/test/echo_canceller3_config_json_unittest.cc
rename to modules/audio_processing/test/echo_canceller3_config_json_unittest.cc
index 4146dda..18c7e9f 100644
--- a/api/audio/test/echo_canceller3_config_json_unittest.cc
+++ b/modules/audio_processing/test/echo_canceller3_config_json_unittest.cc
@@ -8,7 +8,7 @@
* be found in the AUTHORS file in the root of the source tree.
*/
-#include "api/audio/echo_canceller3_config_json.h"
+#include "modules/audio_processing/test/echo_canceller3_config_json.h"
#include "api/audio/echo_canceller3_config.h"
#include "test/gtest.h"
@@ -36,8 +36,12 @@
cfg.multi_channel.stereo_detection_threshold += 1.0f;
cfg.multi_channel.stereo_detection_timeout_threshold_seconds += 1;
cfg.multi_channel.stereo_detection_hysteresis_seconds += 1;
+
std::string json_string = Aec3ConfigToJsonString(cfg);
- EchoCanceller3Config cfg_transformed = Aec3ConfigFromJsonString(json_string);
+ EchoCanceller3Config cfg_transformed;
+ bool parsing_successful;
+ Aec3ConfigFromJsonString(json_string, &cfg_transformed, & parsing_successful);
+ ASSERT_TRUE(parsing_successful);
// Expect unchanged values to remain default.
EXPECT_EQ(cfg.ep_strength.default_len,
diff --git a/rtc_base/BUILD.gn b/rtc_base/BUILD.gn
index 6aff8cc..6abae80 100644
--- a/rtc_base/BUILD.gn
+++ b/rtc_base/BUILD.gn
@@ -855,8 +855,8 @@
}
rtc_library("rtc_json") {
+ testonly = true
public_configs = [ ":rtc_json_suppressions" ]
- poisonous = [ "rtc_json" ]
defines = []
sources = [
"strings/json.cc",
diff --git a/test/BUILD.gn b/test/BUILD.gn
index e7eaa09..be8ee16 100644
--- a/test/BUILD.gn
+++ b/test/BUILD.gn
@@ -1337,6 +1337,8 @@
}
rtc_library("call_config_utils") {
+ testonly = true
+
# TODO(bugs.webrtc.org/10814): Remove rtc_json_suppressions as soon as it
# gets removed upstream.
public_configs = [ "../rtc_base:rtc_json_suppressions" ]
diff --git a/test/fuzzers/BUILD.gn b/test/fuzzers/BUILD.gn
index 114deb0..3d2ac7a 100644
--- a/test/fuzzers/BUILD.gn
+++ b/test/fuzzers/BUILD.gn
@@ -519,7 +519,7 @@
deps = [
":fuzz_data_helper",
"../../api/audio:aec3_config",
- "../../api/audio:aec3_config_json",
+ "../../modules/audio_processing:aec3_config_json",
]
dict = "//testing/libfuzzer/fuzzers/dicts/json.dict"
seed_corpus = "corpora/aec3-config-json-corpus"
diff --git a/test/fuzzers/aec3_config_json_fuzzer.cc b/test/fuzzers/aec3_config_json_fuzzer.cc
index 626350c..b2ded1b 100644
--- a/test/fuzzers/aec3_config_json_fuzzer.cc
+++ b/test/fuzzers/aec3_config_json_fuzzer.cc
@@ -11,7 +11,7 @@
#include <string>
#include "api/audio/echo_canceller3_config.h"
-#include "api/audio/echo_canceller3_config_json.h"
+#include "modules/audio_processing/test/echo_canceller3_config_json.h"
#include "test/fuzzers/fuzz_data_helper.h"
namespace webrtc {
diff --git a/webrtc.gni b/webrtc.gni
index 5a1c43c..173d66c 100644
--- a/webrtc.gni
+++ b/webrtc.gni
@@ -480,9 +480,6 @@
# Default echo detector implementation.
"default_echo_detector",
- # JSON parsing should not be needed in the "slim and modular" WebRTC.
- "rtc_json",
-
# Software video codecs (VP8 and VP9 through libvpx).
"software_video_codecs",
]
